Closed Bug 1530408 Opened 5 years ago Closed 4 years ago

filterResponseData from the WebRequest API returns script bytecode cache data

Categories

(WebExtensions :: Request Handling, defect, P2)

Product:
WebExtensions
Component:
Request Handling
Version:
67 Branch
Type:
defect

Tracking

(firefox-esr68 wontfix, firefox73 wontfix, firefox74 wontfix, firefox75 wontfix, firefox76 wontfix, firefox77 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla77
Tracking Flags:
Tracking Status
firefox-esr68 --- wontfix
firefox73 --- wontfix
firefox74 --- wontfix
firefox75 --- wontfix
firefox76 --- wontfix
firefox77 --- fixed

People

(Reporter: cgemici, Assigned: mixedpuppy)

References

Details

(Keywords: dev-doc-complete, regression)

Attachments

(9 files, 2 obsolete files)

7GbJBWFemd.gif
5.13 MB, image/gif
Details
manifest.zip
864 bytes, application/x-zip-compressed
Details
filterResponseData cache enabled.gif
9.92 MB, image/gif
Details
Workaround for bug 1530408
1.00 KB, application/x-zip-compressed
Details
filterFail.xpi
1.01 KB, application/x-xpinstall
Details
Bug 1530408 disconnect streamfilter and send error if using cached alt-data in request
47 bytes, text/x-phabricator-request
Details | Review
Bug 1530408 disable alt-data if possible when creating a StreamFilter
47 bytes, text/x-phabricator-request
Details | Review
Bug 1530408 add api on channels to prevent using alt-data cache
47 bytes, text/x-phabricator-request
Details | Review
Bug 1530408 fix data race setting StreamFilter disconnected
47 bytes, text/x-phabricator-request
Details | Review

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:67.0) Gecko/20100101 Firefox/67.0

Steps to reproduce:

Noticed this while trying to write a separate extension that needed to read the page response from an extension but the issue can be reproduced with the example at https://github.com/mdn/webextensions-examples/tree/master/http-response

In background.js, to see the problem you can add something like console.log(details.url, str) inside the ondata handler.

In line 20 change the url to the site you want to test this with and the types to ["main_frame", "script"]

Add permissions to the site being used inside manifest.json

When you go to a website, like mozilla.org, the main_frame response will display correctly but some of the scripts seem to contain illegible characters instead. (The first on mozilla was https://www.mozilla.org/media/js/BUNDLES/site.ec263d31b78e.js )

If you open the directly with the url it gets displayed correctly.

Actual results:

When trying to get the response string using the filterResponseData function the response sometimes comes out garbled for script (and potentially other types) but when it works withot any problem for main_frame type requests. Based on some of the comments in https://bugzilla.mozilla.org/show_bug.cgi?id=1255894 I suspect I'm getting the gzipped response.

Expected results:

I should be able to get the response text for any page that's being requested.

Product: Firefox → WebExtensions

When trying to get the response string using the filterResponseData function the response sometimes comes out garbled for script

I also suffered from this problem. My guess is not about gzipped, but cached. There is nothing wrong when the script is not cached and download from streaming, but when the script is cached, the decoding attempt to filterResponseData will be failed. (This is tested by enabling the option "Disable cache")

Possibly related link (not filed in Bugzilla):
https://stackoverflow.com/questions/52128329/decode-string-from-typedarray-cached-data-passed-via-webrequest-filterresponseda
https://discourse.mozilla.org/t/filterresponsedata-bad-decoding-from-cached-file/31417

:mixedpuppy, could you help here ?

Has STR: --- → yes
Component: Untriaged → Request Handling
Flags: needinfo?(mixedpuppy)

Just an FYI that I have tried looking into this, I have yet to make a clear STR for it. I'll continue on it soon.

Flags: needinfo?(mixedpuppy)
Priority: -- → P2

This bug just broke embedded YouTube videos for users of the add-on Iridium (12k users) . At least that's what the developer of the add-on is suspecting.

Maybe this thread will help with reproducing what's happening:
https://github.com/ParticleCore/Iridium/issues/782

I have a workaround for this bug:

function listener(details) {
  if (!details.url.includes("?")) {
    return {
      redirectUrl: details.url + "?" + details.requestId
    };
  }
  else {
    // not already handled?
    if (!(details.url.includes("?" + details.requestId) ||
         details.url.includes("&" + details.requestId))) {
      return {
        redirectUrl: details.url + "&" + details.requestId
      };
    }
  }
  
  let filter = browser.webRequest.filterResponseData(details.requestId);
  let encoder = new TextEncoder();

  let data = [];
  filter.ondata = event => {
    data.push(event.data);
  };

  filter.onstop = async event => {
    let blob = new Blob(data, {type: 'text/javascript'});
    let str = await blob.text();
    console.log(details, str);
    filter.write(encoder.encode(str));
    filter.close();
  };
}

browser.webRequest.onBeforeRequest.addListener(
  listener,
  {urls: ["<all_urls>"], types: ["script"]},
  ["blocking"]
);

It seems to work fine.

Attached image 7GbJBWFemd.gif —
Attached file manifest.zip —
Flags: needinfo?(alexandru.cornestean)

Can you reproduce this bug?

Hello @kernp25,
I managed to reproduce the bug you showed in the attached gif from Comment 6. The issue was reproduced on the latest Nightly (75.0a1/20200303214128), Beta (74.0/20200302184608), Release (73.0.1/20200217142647) under Windows 10 Pro 64-bit and MacOS Catalina 10.15.

As STR, I employed the following:

  1. Load the extension from Comment 7 in about:debugging
  2. Open the extension console (Inspect add-on in about:debugging→ Console)
  3. Open a new tab and access any webpage (for my attempts I used https://www.t-online.de/)
  4. Switch to the add-on console tab
    Note: When first accessing the webpage (url was manually input in the url bar), all console output content is properly displayed. No illegible characters are displayed.
  5. Right click the tab with the previously accessed webpage and reload the tab
  6. Notice that the console output is not correctly displayed, as it contains illegible characters

As per the suggestion from Comment 1, I have also disabled the cache by flipping the browser.cache.disk.enable and browser.cache.memory.enable prefs to false and attempted to reproduce the issue again. This time, console output was properly displayed each time.

Please let me know if there is something else I can assist you with.

Flags: needinfo?(alexandru.cornestean) → needinfo?(kernp25)

@kernp25, what part of the workaround addresses the problem? Is it a redirection and you're tracking the request id to handle the filtering? Or is is something in the filtering itself?

(In reply to Shane Caraveo (:mixedpuppy) from comment #11)

@kernp25, what part of the workaround addresses the problem? Is it a redirection and you're tracking the request id to handle the filtering? Or is is something in the filtering itself?

I'm just Bypassing the cache.

So the url is not cached before adding the filter.

(In reply to Alex Cornestean from comment #9)

Please let me know if there is something else I can assist you with.

Can you set the status of this bug to NEW?

Flags: needinfo?(kernp25)

Hello,

I’ve checked for a regression window (from 01.01.2018 up until the current date) and observed the following:

  • builds from before the first half of May 2019 output the TypeError: blob.text is not a function error in the console while having the extension from Comment 7 installed and opening https://www.t-online.de/ . No other information is displayed in the console apart from that error. This appears to be correlated with Firefox APIs accepting Blob objects with FX69 (05.20.2019)
  • builds after May 2019 are all affected by the issue

Considering these, the issue does not seem to be a regression and will set it to New.

Status: UNCONFIRMED → NEW
status-firefox73: --- → affected
status-firefox74: --- → affected
status-firefox75: --- → affected
Ever confirmed: true

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression

Shane, could we get an assignee to work on this one?

Flags: needinfo?(mixedpuppy)

keeping my ni to look at again, but wontfix'ing for 75 since it's been a couple releases, and there is currently a work around.

status-firefox75: affected → wontfix
status-firefox76: --- → affected
Keywords: regression

I hope, it will be fixed in Firefox 76.

It looks like this should have been fixed by bug 1590898. Looking at the patch there, the test reflects exactly what is happening, but it doesn't test a cached request.

I researched this and am finding that it is always from the cache with a content encoding (I've seen "br" and "gzip").

I'm trying to write a test for this now since I understand what is happening, though it is proving somewhat elusive. Hopefully I'll have that today.

Flags: needinfo?(mixedpuppy) → needinfo?(matt.woodrow)
Keywords: regression
Regressed by: 1583700

And I suspect the change in StreamFilterParent::OnStartRequest in https://phabricator.services.mozilla.com/D50898 is the culprit.

(In reply to Shane Caraveo (:mixedpuppy) from comment #20)

It looks like this should have been fixed by bug 1590898. Looking at the patch there, the test reflects exactly what is happening, but it doesn't test a cached request.

I researched this and am finding that it is always from the cache with a content encoding (I've seen "br" and "gzip").

I'm trying to write a test for this now since I understand what is happening, though it is proving somewhat elusive. Hopefully I'll have that today.

Are you sure about the regressing bug? This bug has been around since way before bug 1583700 landed.

Flags: needinfo?(matt.woodrow)

The test I've attached illustrates a script file with content encoding gzip, however, it is NOT failing. The manual STR does fail.

Attached file filterFail.xpi —

This is a modification on the prior addon that just makes it easier to see the failing requests along with the associated request data.

There is one more issue where filterResponseData fails after redirection (301, 302) - uBlock Origin issue tracker: https://github.com/uBlockOrigin/uBlock-issues/issues/942

Gorhill said:

The StreamObject created by uBO fires a Channel redirected error, and as a result uBO is unable to act on the stream data.

From what I see it works fine in 68.6.0esr and does not in 74.

I'm trying to bisect, but have problem - there is a range where it does not work at all on case with compressed response https://bit.ly/2Wqw7Ol.

I will create new bug when I finish.

This one doesn't seem regressed by 1583700, it is definitely cache related.

Honza, the data we get in onDataAvailable seems to be corrupted or something, it is 100% reproducible, but I haven't figured out why. It is not related to a redirect. Any thoughts?

Flags: needinfo?(honzab.moz)
No longer regressed by: 1583700

Old add-ons did this to read the data:

var binaryInputStream = Cc["@mozilla.org/binaryinputstream;1"].createInstance(Ci.nsIBinaryInputStream);
binaryInputStream.setInputStream(inputStream);
this.data += binaryInputStream.readBytes(count);

I tested it again using the code from here in browser console and the problem also exist here.

Attached file test.js (obsolete) —

The code i used for testing in the browser console.

Ok, I've tracked this down to the bytecode cache. Specifically, if you turn off the pref dom.script_loader.bytecode_cache.enabled .

@kernp25, does this happen for anything other than script loads for you? Does flipping that pref fix the issue (it does for me)?

Some more selective logging led me to this.

nsHttpChannel::PreferAlternativeDataType type(javascript/moz-bytecode-20200325173538) contentType() altData(1)
console.log: "onHeadersRecieved for 252 creating filter\n"
console.log: "onHeadersRecieved for 252 creating filter\n"
HttpChannelParent::OnStartRequest [this=0x148aafc60, aRequest=0x1469fc040]
HttpBaseChannel::SetApplyConversion [this=0x1469fc000 id=188 value=0]
HttpBaseChannel::DoApplyContentConversions [this=0x1469fc000 id=188]
not applying conversion per mApplyConversion id=188
HttpChannelChild::OnStartRequest [this=0x18bfc2000 id=188 applyConversion=1]
HttpBaseChannel::SetApplyConversion [this=0x18bfc2030 is=188 value=1]
HttpBaseChannel::DoApplyContentConversions [this=0x18bfc2030 id=188]
not applying conversion because delivering alt-data
StreamFilterParent::OnDataAvailable
StreamFilterChild::EmitData
StreamFilter::FireDataEvent channelId(252)
console.log: "ondata for 252 [object ArrayBuffer]"
console.log: "ondata invalid chars for 252"

@valentine, in bug 1487100 you added the ability to get at the original input stream. I'm not certain that will help in this case, where we're using a stream filter. It seems like I need a way to signal for a channel to bypass the alt-data[1]. I'm not sure if there is a way to handle this just for the extension stream filters, allowing the byte cache to be used otherwise, or if there is a way to decode within our own OnDataAvailable handler[2].

[1] https://searchfox.org/mozilla-central/rev/4d9cd186767978a99dafe77eb536a9525980e118/netwerk/protocol/http/HttpBaseChannel.cpp#1182-1186
[2] https://searchfox.org/mozilla-central/rev/4d9cd186767978a99dafe77eb536a9525980e118/toolkit/components/extensions/webrequest/StreamFilterParent.cpp#604

Flags: needinfo?(valentin.gosu)
Summary: filterResponseData from the WebRequest API returns unexpected data → filterResponseData from the WebRequest API returns script bytecode cache data

@valentine, also is there any magic to making this happen for the test I attached?

Assignee: nobody → mixedpuppy

Thanks for tracking this down. Seems a bit tricky indeed.
From what I can tell, we have 2 options here:

  1. Trying to clear the preferred alt-data types when there is a filter installed for the request.
    I am not sure we can do that (depending on how filterResponseData works)
    • can we know if the filter is installed for a channel before calling async open / before onStartRequest ?
  2. Maybe we could use the original input stream - again, I'm not sure how the filter works - technically we could open the original input stream, ignore all the onDataAvailable calls, and just read from the stream and pass that forward.
Flags: needinfo?(valentin.gosu)
Flags: needinfo?(honzab.moz)

(In reply to Shane Caraveo (:mixedpuppy) from comment #32)

Does flipping that pref fix the issue (it does for me)?

When using the add-on from Comment 7 and flipping the pref it fixes the issue for me.

(In reply to Valentin Gosu [:valentin] (he/him) from comment #34)

Thanks for tracking this down. Seems a bit tricky indeed.
From what I can tell, we have 2 options here:

  1. Trying to clear the preferred alt-data types when there is a filter installed for the request.
    I am not sure we can do that (depending on how filterResponseData works)

It implements nsIStreamListener. If there is a way to clear the alt-data types (only for the channel being attached to) I could do that just after successfully calling channel.setNewListener. This seems like it would be the cleanest way to deal with this (based on my thinking through my responses below).

  • can we know if the filter is installed for a channel before calling async open / before onStartRequest ?

Not before asyncOpen, but certainly before onStartRequest since streamfilter implements that. What I see is that the alt-data input stream is opened prior to the response starting (webRequest.onHeadersReceived) but after http-on-modified (webRequest.onBeforeRequest). So relying on that would require extensions to implement their filter during onBeforeRequest. That may not be optimal for some situations.

In the case an extension creates the filter earl (onBeforeRequest) we could probably identify that an extension has a stremlistener attached and bypass alt-data for that request.

If we can somehow change the input stream prior to calling nsIStreamListener.onStartRequest, we could probably also do the same (detect that an extension has attached a streamlistener).

  1. Maybe we could use the original input stream - again, I'm not sure how the filter works - technically we could open the original input stream, ignore all the onDataAvailable calls, and just read from the stream and pass that forward.

DoApplyContentConversions is called right after StreamFilterParent::OnStartRequest, so we could conceivably bypass alt-data somehow during StreamFilterParent::OnStartRequest. Perhaps signal the channel to switch back to the original stream? As mentioned above, at this point the alt data stream is already opened.

Flags: needinfo?(valentin.gosu)

Ok, I have a solution, though kinda hacky IMO.

We need to stop the block that decides to load alt-data[1]. I can do that by inserting the following before attaching our stream listener[2]. If I wait till later (when we call setNewListener) the cache decision has already won that race. Note that this also requires the extension to create the filter in onBeforeRequest, rather than onHeadersReceived. It seems to me that this could also be a performance hit (bypassing the bytecode cache).

  // Disable alt-data for extension stream listeners.
  nsCOMPtr<nsIHttpChannelInternal> internal = do_QueryInterface(chan);
  internal->SetAltDataForChild(false);

[1] https://searchfox.org/mozilla-central/rev/b712398b7fae54ef377a558d6f16dede7a7f8530/netwerk/protocol/http/nsHttpChannel.cpp#5122-5132
[2] https://searchfox.org/mozilla-central/rev/b712398b7fae54ef377a558d6f16dede7a7f8530/toolkit/components/extensions/webrequest/StreamFilterParent.cpp#139

(In reply to Shane Caraveo (:mixedpuppy) from comment #37)

Ok, I have a solution, though kinda hacky IMO.

We need to stop the block that decides to load alt-data[1]. I can do that by inserting the following before attaching our stream listener[2]. If I wait till later (when we call setNewListener) the cache decision has already won that race. Note that this also requires the extension to create the filter in onBeforeRequest, rather than onHeadersReceived. It seems to me that this could also be a performance hit (bypassing the bytecode cache).

Presumably there's no way of knowing if the filter will modify the response, and if that's the case, we definitely don't want to deliver alt-data.
Alt-data is already an optimization, so I am generally OK with this solution.

DoApplyContentConversions is called right after StreamFilterParent::OnStartRequest, so we could conceivably bypass alt-data somehow during StreamFilterParent::OnStartRequest. Perhaps signal the channel to switch back to the original stream? As mentioned above, at this point the alt data stream is already opened.

Switching back to the original stream would be technically possible, but I don't think it's possible to do from onStartRequest, since we've already started to read from the stream by them. Plus, if we want ContentConversions to happen, they would need to happen on the original content.

Flags: needinfo?(valentin.gosu)

(In reply to Valentin Gosu [:valentin] (he/him) from comment #38)

(In reply to Shane Caraveo (:mixedpuppy) from comment #37)

Ok, I have a solution, though kinda hacky IMO.

We need to stop the block that decides to load alt-data[1]. I can do that by inserting the following before attaching our stream listener[2]. If I wait till later (when we call setNewListener) the cache decision has already won that race. Note that this also requires the extension to create the filter in onBeforeRequest, rather than onHeadersReceived. It seems to me that this could also be a performance hit (bypassing the bytecode cache).

Presumably there's no way of knowing if the filter will modify the response, and if that's the case, we definitely don't want to deliver alt-data.
Alt-data is already an optimization, so I am generally OK with this solution.

We do not have any way to know. So I think the solution here will generally be:

  • SetAltDataForChild early
  • determine what type we have, if it is alt data, disconnect StreamFilter and send an error to the extension.
    • disconnect because we do not want an extension modifying any alt-data that gets used.

With the above, an extension can create the filter during onBeforeRequest if it wants/needs to modify cached scripts/etc that would use alt data. If it creates the filter later, we wont give the extension any of that data in the filter.

(In reply to Shane Caraveo (:mixedpuppy) from comment #39)

With the above, an extension can create the filter during onBeforeRequest if it wants/needs to modify cached scripts/etc that would use alt data. If it creates the filter later, we wont give the extension any of that data in the filter.

I didn't know that was an option. That seems like a good plan! Let me know if you need any help with anything. Thanks!

(In reply to Shane Caraveo (:mixedpuppy) from comment #39)

With the above, an extension can create the filter during onBeforeRequest if it wants/needs to modify cached scripts/etc that would use alt data. If it creates the filter later, we wont give the extension any of that data in the filter.

This needs also be documented on mdn?

uBlock Origin seems to use filterResponseData, so disabling the bytecode cache for everyone with that extension installed would be unfortunate.

(In reply to Tom Schuster [:evilpie] from comment #42)

uBlock Origin seems to use filterResponseData, so disabling the bytecode cache for everyone with that extension installed would be unfortunate.

Presumably this bug only applies to JS files, and only when they are loaded from the cache.
Maybe we could find a way to exclude all the others?

(In reply to Valentin Gosu [:valentin] (he/him) from comment #43)

(In reply to Tom Schuster [:evilpie] from comment #42)

uBlock Origin seems to use filterResponseData, so disabling the bytecode cache for everyone with that extension installed would be unfortunate.

Presumably this bug only applies to JS files, and only when they are loaded from the cache.
Maybe we could find a way to exclude all the others?

Actually, only when loaded from alt-data. js files loaded from cache (not alt-data) work just fine in the filter.

So far as disabling bytecode cache, I was already aware of the issue and considering what might be done for that.

(In reply to Tom Schuster [:evilpie] from comment #42)

uBlock Origin seems to use filterResponseData, so disabling the bytecode cache for everyone with that extension installed would be unfortunate.

uBO is using it only for main document. I think AdGuard is using it on any text response https://kb.adguard.com/en/general/how-to-create-your-own-ad-filters#replace-modifier

Okay very good. I didn't read the code carefully. We should maybe check if there is another popular extension that would break it.

It's not a matter of what uBO does, it's whether we want to make it possible to turn off the byte cache which could affect overall performance.

Right now I'm leaning towards simply throwing a filter.onerror so extensions can detect the situation, and so we avoid the potential for an extension to change the bytecache data.

With the above, an extension can create the filter during onBeforeRequest if it wants/needs to modify cached scripts/etc that would use alt data.

But this will still be possible?

(In reply to kernp25 from comment #49)

With the above, an extension can create the filter during onBeforeRequest if it wants/needs to modify cached scripts/etc that would use alt data.

But this will still be possible?

The point is whether that is something we actually want to allow, and whether we can actually make it work consistently. I'm going to add the error for now since it at least makes the situation better than it currently is.

(In reply to Shane Caraveo (:mixedpuppy) from comment #48)

It's not a matter of what uBO does, it's whether we want to make it possible to turn off the byte cache which could affect overall performance.

It seems to me that if a request is going to be intercepted and maybe modified, then alt-data would be useless anyway, so we can just disable it in that case.

(In reply to Shane Caraveo (:mixedpuppy) from comment #50)

The point is whether that is something we actually want to allow

This does not make sense!

If a add-on wants to read a cached js file, it can do so by using the bypass cache hack!

I'm not interested in the alt-data i just want to read the cached js file.

If the script byte cache is bypassing the normal http cache, we do not want to
offer the ability to an extension to change the cache data.

If an extension creates a stream filter early enough, we can
disable the use of the alt-data cache allowing the extension to properly
handle filtering.

Attachment #9134560 - Attachment is obsolete: true

I've yet to decide whether the 2nd patch is a good idea and will get an additional opinion on our team. In the meantime...

@valentin, if we disabled alt-data and an extension were to use a filter on all requests, it is essentially the same a flipping the pref. Are you certain that doesn't matter?

When using a listener with "<all_urls>", it is not great for performance anyway.

Sorry for OT, but speaking about performance, uBlock Origin is still injecting no-cache, no-store, must-revalidate into cache-control header value of all requests affected by filters which use filterResponseData or modify CSP headers because of cache racing(?) - bug 1376932 which is for some reason (incorrectly) RESOLVED FIXED.

Disconnecting a Streamfilter and sending an error only is useless, because then the add-on just bypasses the cache.

(In reply to Shane Caraveo (:mixedpuppy) from comment #56)

if we disabled alt-data and an extension were to use a filter on all requests

A add-on can NOT filter all request e.g. addons.mozilla.org. So, this is not the same as flipping the pref.

The proposed solution doesn't sit well with me. At least in theory, between a public API we expose to extensions (which have stability guarantees we try to maintain) and the bytecode optimization (which sits on top of regular caching as a basic optimization), stability of the API should win every time.

Also, there doesn't seem to be anything fundamentally un-implementable about the whole setup. Unless I'm misunderstanding things, we should be able to have both an optimized path for the common case, and throwing away the bytecode when extension wants to rewrite a script coming from cache.

(Note that I'm not claiming anything about implementing that being simple or easy, just that the two requirements don't strictly cancel each other out, again at least in theory)

In that light, this looks more like a temporary hack than a real solution. Since it looks like extensions that really want to do inspection/rewriting of scripts already work around the current problem via cache busting, they are likely to continue to do so even if we land this "fix", so I'm not sure what this approach really helps with.

That last point is particularly important, since in addition to bytecode caching, extensions are effectively also disabling regular caching for scripts (and possibly other resources, unintentionally). However, if we are able to give extensions specific control of this, perhaps with an additional extraInfoSpec param like "skipBytecodeCache", then in combination with this proposal (making the FilterResponse throw for scripts without that flag), we might actually be in a better place.

Longer term, I think we should actually aim to have bytecode cache implementation better support the usecase of extensions, but I'll let Valentin speak about feasibility of that.

(In reply to Tomislav Jovanovic :zombie from comment #61)

In that light, this looks more like a temporary hack than a real solution. Since it looks like extensions that really want to do inspection/rewriting of scripts already work around the current problem via cache busting, they are likely to continue to do so even if we land this "fix", so I'm not sure what this approach really helps with.

Maybe you can add something like this in the details object: fromBytecodeCache: false

Valentin indicated he wasn't concerned that we disable alt-data, though it bothers me a touch, I'm going to go with what he said. So, we do not need any change in the api to get something that will work. At some point, perhaps we'll get some cache work done that will allow this to work when creating the filter in onHeadersReceived.

Pushed by scaraveo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3c76ac7b3965
disconnect streamfilter and send error if using cached alt-data in request r=valentin,zombie
https://hg.mozilla.org/integration/autoland/rev/77bb11a76b33
add api on channels to prevent using alt-data cache r=valentin
https://hg.mozilla.org/integration/autoland/rev/365c4d5fe9b0
disable alt-data if possible when creating a StreamFilter r=valentin,zombie

This should have some additional notes on the mdn docs for filterResponseData. Something along the lines:

Firefox uses an optimized byte cache for script requests that overrides the normal request caching. Data from this alternate cache is not available in a useful form for extensions. If your extension needs to filter scripts, you should create your filter in onBeforeRequest so that the filter is created prior to the attempt to load from cache. In this case, we bypass the alternate cache used for scripts, for the request being handled.

Keywords: dev-doc-needed

Thanks for fixing this annoying bug!

Backout merged: https://hg.mozilla.org/mozilla-central/rev/df39286e09e7

Status: RESOLVED → REOPENED
status-firefox77: fixed → affected
Resolution: FIXED → ---
Target Milestone: mozilla77 → ---

Can this fix be uplifted to Firefox 76?

Blocks: 1629080

mDisconnected should only be changed on the main thread.

Pushed by scaraveo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/268925ed4b5c
disconnect streamfilter and send error if using cached alt-data in request r=valentin,zombie
https://hg.mozilla.org/integration/autoland/rev/8bcb78e10e86
add api on channels to prevent using alt-data cache r=valentin
https://hg.mozilla.org/integration/autoland/rev/79632a1a297c
disable alt-data if possible when creating a StreamFilter r=valentin,zombie
https://hg.mozilla.org/integration/autoland/rev/5bf3acec2165
fix data race setting StreamFilter disconnected r=zombie
Flags: needinfo?(mixedpuppy)

The patch landed in nightly and beta is affected.
:mixedpuppy, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(mixedpuppy)

Considering this issue has been around for a year I'll be more conservative with the fix and let it ride the train.

status-firefox76: affected → wontfix
Flags: needinfo?(mixedpuppy)
Attachment #9135461 - Attachment is obsolete: true

(In reply to Shane Caraveo (:mixedpuppy) from comment #69)

This should have some additional notes on the mdn docs for filterResponseData. Something along the lines:

Firefox uses an optimized byte cache for script requests that overrides the normal request caching. Data from this alternate cache is not available in a useful form for extensions. If your extension needs to filter scripts, you should create your filter in onBeforeRequest so that the filter is created prior to the attempt to load from cache. In this case, we bypass the alternate cache used for scripts, for the request being handled.

Can this page also be updated?

It says: Code that is loaded to be executed by a <script> element

But it will only be called, if the script element has a src attribute.
It will not be called for a script element without a src attribute.

I've added a note to webRequest.filterResponseData() in line with the suggestion in https://bugzilla.mozilla.org/show_bug.cgi?id=1530408#c69. Please let me know if this is OK. I assume no release notes update is necessary.

Flags: needinfo?(mixedpuppy)

Thanks!

Flags: needinfo?(mixedpuppy)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: