1262359 - crash in OOM | large | NS_ABORT_OOM | nsACString_internal::Assign | nsDataHandler::NewURI

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Nicholas Nethercote [inactive]]

This bug was filed from the Socorro interface and is 
report bp-cd7d10be-a12e-4ad3-a18e-cefb92160405.
=============================================================

nsDataHandler::NewURI() does an allocation (via nsDataHandler::ParseURI()) that is the size of the payload of a data URL. This can be arbitrarily long, so it should be made fallible. (The occurrence in this crash report was 1.4 MB.)

Also, nsDataHandler::ParseURI() can be optimized a bit to avoid the allocation entirely in some cases.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Remove unused |hashRef| parameter from nsDataChannel::ParseURI()

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Make the filling in of two parameters optional in nsDataChannel::ParseURI()

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Add a missing fallible nsTSubstring_CharT::Assign variant

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Make data URL payload assignment fallible in nsDataChannel::ParseURI()

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 5) - Add a missing rv check for call to nsDataHandler::ParseURI()

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Remove unused |hashRef| parameter from nsDataHandler::ParseURI()

Comment 7

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Make the filling in of two parameters optional in nsDataHandler::ParseURI()

Comment 8

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Make data URL payload assignment fallible in nsDataHandler::ParseURI()

Comment 9

[Jason Duell]

Comment on attachment 8738421 [details] [diff] [review]
(part 4) - Make data URL payload assignment fallible in nsDataHandler::ParseURI()

Review of attachment 8738421 [details] [diff] [review]:
-----------------------------------------------------------------

bonus points for snazzy use of ternary operator! :)

Comment 10

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b2581d348367fcad75830e693d8086bff16de572
Bug 1262359 (part 1) - Remove unused |hashRef| parameter from nsDataHandler::ParseURI(). r=jduell.

https://hg.mozilla.org/integration/mozilla-inbound/rev/ee3b6b151cf5a72ab0a121c0f3da475129ba7c29
Bug 1262359 (part 2) - Make the filling in of two parameters optional in nsDataHandler::ParseURI(). r=jduell.

https://hg.mozilla.org/integration/mozilla-inbound/rev/6474dc4bf7856b5583ffea9307a7bdc249bf8bc8
Bug 1262359 (part 3) - Add a missing fallible nsTSubstring_CharT::Assign() variant. r=erahm.

https://hg.mozilla.org/integration/mozilla-inbound/rev/fb3e2cc58cfa53b11e7e89d418abd2db1845a7d6
Bug 1262359 (part 4) - Make data URL payload assignment fallible in nsDataHandler::ParseURI(). r=jduell.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e8dad24cfffa9f5d8406df8c59839902cd446e65
Bug 1262359 (part 5) - Add a missing rv check for call to nsDataHandler::ParseURI(). r=jduell.

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b2581d348367
https://hg.mozilla.org/mozilla-central/rev/ee3b6b151cf5
https://hg.mozilla.org/mozilla-central/rev/6474dc4bf785
https://hg.mozilla.org/mozilla-central/rev/fb3e2cc58cfa
https://hg.mozilla.org/mozilla-central/rev/e8dad24cfffa

Comment 12

[Marco Castelluccio [:marco]]

Looks like these changes fixed the crash in bug 1258111.