Skip to content

bitem::parse: fixup uncaught exception.#5

Merged
Frederick888 merged 1 commit into
Frederick888:masterfrom
ttys3:master
Jun 9, 2019
Merged

bitem::parse: fixup uncaught exception.#5
Frederick888 merged 1 commit into
Frederick888:masterfrom
ttys3:master

Conversation

@ttys3

@ttys3 ttys3 commented May 30, 2019

Copy link
Copy Markdown
Contributor
std::stoull may throw an exception cause php terminated.
for example:
    bitem::parse('d5:infod6:lengthi364e4:name8:test.rare3:inti1548400939e4:listl5:UTF-83:GBKe6:string13:uTorrent/3130e');
exception:
    terminate called after throwing an instance of 'std::invalid_argument'

@Frederick888

Copy link
Copy Markdown
Owner

Thanks for the PR!

Hmmm... I think bitem::load needs the same fix and perhaps it makes sense to catch std::out_of_range as well. Would you like to make the changes or you want me to do this afterwards?

std::string::substr may cause std::out_of_range exception
std::stoull and std::stoll may cause std::invalid_argument exception
both will cause php terminated.

related method:
bdict::parse
bint::parse
bstr::parse
bitem::escape_key  -> used by bdict::search
bitem::get_current_key  -> used by bdict/blist  del_path get_path set_path
@ttys3
ttys3 force-pushed the master branch 2 times, most recently from a5fa0f3 to 468351e Compare June 3, 2019 06:21
@ttys3

ttys3 commented Jun 3, 2019

Copy link
Copy Markdown
Contributor Author

I've made a macro wrap named CALL_AND_HANDLE to handle the exception.

std::string::substr may cause std::out_of_range exception
std::stoull and std::stoll may cause std::invalid_argument exception
both will cause php terminated.

related method:
bdict::parse
bint::parse
bstr::parse
bitem::escape_key -> used by bdict::search
bitem::parse
bitem::load

maybe related:
bitem::get_current_key -> used by bdict/blist del_path get_path set_path

@Frederick888
Frederick888 merged commit 468351e into Frederick888:master Jun 9, 2019
@Frederick888

Copy link
Copy Markdown
Owner

The macro looks good to me :) I'll publish a new release asap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants