Closed Bug 1354508 Opened 8 years ago Closed 8 years ago

Add filter option for network requests checking for a specific response header

Categories

(DevTools :: Netmonitor, enhancement)

55 Branch
enhancement
Not set
normal

Tracking

(firefox55 verified)

VERIFIED FIXED
Firefox 55
Tracking Status
firefox55 --- verified

People

(Reporter: sebo, Assigned: vkatsikaros)

References

Details

(Keywords: dev-doc-complete, good-first-bug)

Attachments

(2 files)

The filter field within the Network Monitor should support a filter option that checks the requests for a specific HTTP header in the response. The Chrome DevTools provide a 'has-response-header' filter for this purpose. Sebastian
See Also: → 1354509
Blocks: netmonitor-filtering
No longer blocks: 1041895
Keywords: good-first-bug
I think I know what to tweak for this bug. Could I have this assigned? Should I use the same filter name, ie 'has-response-header'?
Flags: needinfo?(ntim.bugs)
Assignee: nobody → vkatsikaros
Flags: needinfo?(ntim.bugs)
(In reply to Vangelis Katsikaros from comment #1) > I think I know what to tweak for this bug. Could I have this assigned? I've given you permission to assign yourself to bugs, so you can do it on any bug next time :) > Should I use the same filter name, ie 'has-response-header'? Yes, has-response-header.
Thanks Tim!
Attached image has-response-header.png
Comment on attachment 8858114 [details] Bug 1354508 - Add filter option for network requests checking for a specific response header. https://reviewboard.mozilla.org/r/130084/#review132708 Looks good generally, thanks for working on this! I have a few questions before granting a ship-it. ::: devtools/client/netmonitor/src/utils/filter-text-utils.js:133 (Diff revision 1) > break; > case "remote-ip": > match = `${item.remoteAddress}:${item.remotePort}`.toLowerCase().includes(value); > break; > + case "has-response-header": > + if (typeof item.responseHeaders === "object") { Does item.hasOwnProperty("responseHeaders") work instead ? or does that omit some cases ? ::: devtools/client/netmonitor/src/utils/filter-text-utils.js:134 (Diff revision 1) > + let responseHeaders = item.responseHeaders.headers; > + let r = responseHeaders.findIndex((e) => { > + return e.name.toLowerCase() === value; > + }); You can simplify this a bit: let { headers } = item.responseHeaders; match = headers.findIndex(h => h.name.toLowerCase() === value) > -1; ::: devtools/client/netmonitor/src/utils/filter-text-utils.js:134 (Diff revision 1) > + let responseHeaders = item.responseHeaders.headers; > + let r = responseHeaders.findIndex((e) => { > + return e.name.toLowerCase() === value; > + }); You can simplify this a bit: let { headers } = item.responseHeaders; match = headers.findIndex(h => h.name.toLowerCase() === value) > 0; ::: devtools/client/netmonitor/src/utils/filter-text-utils.js:138 (Diff revision 1) > + if (typeof item.responseHeaders === "object") { > + let responseHeaders = item.responseHeaders.headers; > + let r = responseHeaders.findIndex((e) => { > + return e.name.toLowerCase() === value; > + }); > + match = r > 0; Shouldn't that be `> -1` instead of `> 0`? Otherwise, if the first index is matched, it will return false. Or am I missing something here (eg. the first item has a special meaning?) ?
Comment on attachment 8858114 [details] Bug 1354508 - Add filter option for network requests checking for a specific response header. https://reviewboard.mozilla.org/r/130084/#review132708 > Does item.hasOwnProperty("responseHeaders") work instead ? or does that omit some cases ? This twisted my brain a bit, and I apologize for missing something obvious. With: ``` @@ -128,6 +129,19 @@ function isFlagFilterMatch(item, { type, case "remote-ip": match = `${item.remoteAddress}:${item.remotePort}`.toLowerCase().includes(value); break; + case "has-response-header": +console.log( typeof item); +console.log( typeof item.responseHeaders); +console.log( typeof item.responseHeaders.headers); +console.log( typeof item.responseHeaders === "object"); +console.log(item.hasOwnProperty("responseHeaders")); + if (item.hasOwnProperty("responseHeaders")) { + let { headers } = item.responseHeaders; + match = headers.findIndex(h => h.name.toLowerCase() === value) > -1; + } else { + match = false; + } + break; ``` I get in the browser console (if I filter the network panel) the output: ``` object filter-text-utils.js:133:1 object filter-text-utils.js:134:1 object filter-text-utils.js:135:1 true filter-text-utils.js:136:1 false filter-text-utils.js:137:1 ``` with `item.hasOwnProperty("responseHeaders")` giving `false` whereas the key exists in the object. I initially decided to workaround this, but it seems there is a lesson to be learned here, so I was wondering if I could get some feedback. > You can simplify this a bit: > > let { headers } = item.responseHeaders; > match = headers.findIndex(h => h.name.toLowerCase() === value) > -1; Neater > Shouldn't that be `> -1` instead of `> 0`? Otherwise, if the first index is matched, it will return false. > > Or am I missing something here (eg. the first item has a special meaning?) ? Ah! I meant to type ">= 0" (but didn't) :/ However, your suggestion hints more cleanly to the return value of findIndex
Comment on attachment 8858114 [details] Bug 1354508 - Add filter option for network requests checking for a specific response header. https://reviewboard.mozilla.org/r/130084/#review132916 r+ with my 2 comments addressed.
Attachment #8858114 - Flags: review?(ntim.bugs) → review+
Comment on attachment 8858114 [details] Bug 1354508 - Add filter option for network requests checking for a specific response header. https://reviewboard.mozilla.org/r/130084/#review132708 > This twisted my brain a bit, and I apologize for missing something obvious. > > With: > > ``` > @@ -128,6 +129,19 @@ function isFlagFilterMatch(item, { type, > case "remote-ip": > match = `${item.remoteAddress}:${item.remotePort}`.toLowerCase().includes(value); > break; > + case "has-response-header": > +console.log( typeof item); > +console.log( typeof item.responseHeaders); > +console.log( typeof item.responseHeaders.headers); > +console.log( typeof item.responseHeaders === "object"); > +console.log(item.hasOwnProperty("responseHeaders")); > + if (item.hasOwnProperty("responseHeaders")) { > + let { headers } = item.responseHeaders; > + match = headers.findIndex(h => h.name.toLowerCase() === value) > -1; > + } else { > + match = false; > + } > + break; > ``` > I get in the browser console (if I filter the network panel) the output: > ``` > object filter-text-utils.js:133:1 > object filter-text-utils.js:134:1 > object filter-text-utils.js:135:1 > true filter-text-utils.js:136:1 > false filter-text-utils.js:137:1 > ``` > with `item.hasOwnProperty("responseHeaders")` giving `false` whereas the key exists in the object. > > I initially decided to workaround this, but it seems there is a lesson to be learned here, so I was wondering if I could get some feedback. Sounds fine then, dropping this issue.
Tim, as far as the process is concerned, do I also need to trigger a try, on top of the push to mozreview? Also, I would be curious to find an explanation for the hasOwnProperty behavior in https://bugzilla.mozilla.org/show_bug.cgi?id=1354508#c7 , I really can't figure it out :/
Flags: needinfo?(ntim.bugs)
(In reply to Vangelis Katsikaros from comment #11) > Tim, as far as the process is concerned, do I also need to trigger a try, on > top of the push to mozreview? This patch should be safe enough for tests, so no need for a try push. > Also, I would be curious to find an explanation for the hasOwnProperty > behavior in https://bugzilla.mozilla.org/show_bug.cgi?id=1354508#c7 , I > really can't figure it out :/ Not sure, probably because responseHeaders is not defined as a property but as a getter or something else.
Flags: needinfo?(ntim.bugs)
Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/autoland/rev/6a9ce1980dcc Add filter option for network requests checking for a specific response header. r=ntim
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
I have reproduced this bug with Nightly 55.0a1 (2017-04-07) on Windows 10 , 64 Bit ! This bug's fix is Verified with latest Nightly ! Build ID 20170427030231 User Agent Mozilla/5.0 (Windows NT 10.0; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0 [bugday-20170426]
Status: RESOLVED → VERIFIED
Keywords: dev-doc-needed
Added description of the filter to https://developer.mozilla.org/en-US/docs/Tools/Network_Monitor#Filtering_by_properties and a developer release note to https://developer.mozilla.org/en-US/Firefox/Releases/55#Developer_Tools. Vangelis, can you please have a look at the documentation and let me know whether it's ok? Sebastian
Flags: needinfo?(vkatsikaros)
Thanks Sebastian, it looks fine.
Flags: needinfo?(vkatsikaros)
Great! Thanks for the fast response! Sebastian
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: