Closed Bug 1464648 Opened 2 years ago Closed 2 years ago

Bail early when parsing empty form data (to avoid NS_NOTREACHED assertion failure)

Categories

(Core :: DOM: Core & HTML, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

Details

Attachments

(1 file)

I'm slowly replacing uses of XPCOM's non-fatal NS_NOTREACHED with MFBT's fatal MOZ_ASSERT_UNREACHABLE. This NS_NOTREACHED("Should never reach here.") assertion:

https://searchfox.org/mozilla-central/rev/14578d6f2d2ec5246572827061ecefa60a40855d/dom/base/BodyUtil.cpp#390

fails on the following test case of the testing/web-platform/tests/fetch/api/response/response-consume-empty.html web platform test:

checkResponseWithNoBody("formData with correct multipart type (error case)", checkBodyFormDataError, [["Content-Type", 'multipart/form-data; boundary="boundary"']]);

https://searchfox.org/mozilla-central/rev/b47af5169898b156429c8cdaf415afdaa3c43a7b/testing/web-platform/tests/fetch/api/response/response-consume-empty.html#82-83
Comment on attachment 8980951 [details]
Bug 1464648 - Bail early when parsing empty form data (to avoid NS_NOTREACHED assertion failure).

https://reviewboard.mozilla.org/r/247100/#review253432

::: dom/base/BodyUtil.cpp:332
(Diff revision 1)
>      mData.BeginReading(start);
>      // This should ALWAYS point to the end of data.
>      // Helpers make copies.
>      mData.EndReading(end);
>  
> -    while (start != end) {
> +    if (start == end) {

What about this check at the beginning of Parse() ?

if (mData.IsEmpty()) {
  return false;
}
Attachment #8980951 - Flags: review?(amarchesini) → review-
(In reply to Andrea Marchesini [:baku] from comment #2)
> What about this check at the beginning of Parse() ?
> 
> if (mData.IsEmpty()) {
>   return false;
> }

Good point! We don't need to parse mMimeType if there is no form data to parse. :)
Comment on attachment 8980951 [details]
Bug 1464648 - Bail early when parsing empty form data (to avoid NS_NOTREACHED assertion failure).

https://reviewboard.mozilla.org/r/247100/#review254614
Attachment #8980951 - Flags: review?(amarchesini) → review+
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/578f9a189eb8
Bail early when parsing empty form data (to avoid NS_NOTREACHED assertion failure). r=baku
https://hg.mozilla.org/mozilla-central/rev/578f9a189eb8
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.