Integer overflow when parsing atom count in molecular formula

[oatkachenko]

If molecular formula has huge amount of atoms then parser calculates it improperly.

Molecular formula: H4294967296
AR: 0 atoms
ER: 1 atom

Molecular formula: H4294967298
AR: 2 atoms
ER: 1 atom

[johnmay]

That is more atoms than the largest known molecule, I think it's fine.

C169719H270466N45688O52238S911
https://en.wikipedia.org/wiki/Titin

Even if we use a long instead of an integer you are likely to Run out of RAM as the parse actually constructs the atoms.

[oatkachenko]

I pushed commit before I saw your answer :) Anyway thank you for explanation.

[johnmay]

Almost there, but why not just return out of the loop:

while (isDigit(c = next())) {
   res = (10 * res) + (c - '0');
   if (res < 0) return -1;
}

You do the check after the loop but you can overflow back to 0

[oatkachenko]

In this case, if overflow will happen in the middle of a number - iterator will stay there and we can't get the other part of the string.

[johnmay]

If it overflows it should be an error non? "Unexpected input"

[johnmay]

So the rest of the string doesn't matter, if someone wants to handle formulas with more than 2 billion hydrogens I think they can write their own parser for that :-)

[oatkachenko]

If it overflows it should be an error non? "Unexpected input"

I think it should

while (isDigit(c = next())) {
   res = (10 * res) + (c - '0');
   if (res < 0) return -1;
}

btw, it is possible to overflow from positive to positive e.g. 4294967298 will be 429496729 and then 2

So the rest of the string doesn't matter, if someone wants to handle formulas with more than 2 billion hydrogens I think they can write their own parser for that :-)

Absolutely agree:)

[oatkachenko]

What are the advantages to return 1 if amount of atom is negative value?

MolecularFormulaManipulator#parseIsotope

count = iter.nextUInt();
        if (count < 0)
            count = 1;

Maybe we could return false?

[johnmay]

I'd have to remember what the old code did but yes count < 0 should probably just be an error

[johnmay]

Fixed via #808