967393 - Make the jsonp regex a bit more relaxed

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect)

Description

[Victor Porof [:vporof][:vp]]

I've seen jsonp requests in the wild (like on http://www.nytimes.com/) that don't strictly adhere to the jsonp syntax, but they're still valid javascript, so they work. Our regex to identify those requests is very strict. We should gracefully handle all types of jsonp.

Comment 1

[Victor Porof [:vporof][:vp]]

Attachment: v1

Comment 2

[Victor Porof [:vporof][:vp]]

Attachment: new regex

Visualization for the new regex.

Comment 3

[Victor Porof [:vporof][:vp]]

As a clarification, the first captured group is the jsonp callback, and the second group is the actual json object passed as a param to the callback.

Comment 4

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 8369923 [details] [diff] [review]
v1

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

r+. just add some description to that regex and this is good to go.

::: browser/devtools/netmonitor/netmonitor-view.js
@@ +2138,5 @@
>          jsonObjectParseError = e;
>        }
>        if (jsonMimeType || jsonObject) {
>          // Extract the actual json substring in case this might be a "JSONP".
> +        let jsonpRegex = /^\s*([\w$]+)\s*\(\s*([^]*)\s*\)\s*;?\s*$/;

can you describe what this regex is doing in a comment, please? Having to decode this is cruel.

Comment 5

[Rob Campbell [:rc] (:robcee)]

maybe you can data-uri-encode that png as documentation. ;)

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b96fb0f79460