new Response with additional headers not respected

RESOLVED INCOMPLETE

Status

()

defect
P2
normal
RESOLVED INCOMPLETE
3 years ago
a year ago

People

(Reporter: wheresrhys, Unassigned)

Tracking

(Blocks 1 bug)

48 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36

Steps to reproduce:

The following function takes a Response and a map of headers and returns a new Response with the original Response's headers augmented by the new map. Note that it does not use Response.clone(), but rather creates a wholly new Response object. when running integration tests this function is called on each cors response in order to indicate the response was retrieved from the cache
```
function addHeadersToResponse (res, headers) {
	const response = res.clone()
	const init = {
			status: response.status,
			statusText: response.statusText,
			headers
	};

	response.headers.forEach((v,k) => {
			init.headers[k] = v;
	});
	return response.text()
		.then(body => {
			return new Response(body, init);
		})

}
```




Actual results:

In Chrome this function works as expected. However in firefox 48.0.2 the new Response is created, and logging verifies that this is passed in to `event.respondWith()`, however when the response passed to the page is logged it appears to be the original response retrieved from the cache as the new headers have not been added


Expected results:

Additional headers should have been present in the response in the client page.
(Reporter)

Comment 1

3 years ago
Note: when I said res.clone() is not used, I meant that no attempt is made to mutate the headers of res.clone() and respond with them (which should not work as per the spec)
Component: Untriaged → Networking
Product: Firefox → Core
What kind of headers are you trying to apply?  Is there a URL where I can see this failing?
Flags: needinfo?(wheresrhys)
Component: Networking → DOM: Service Workers
(Reporter)

Comment 3

3 years ago
The header is `FT-Debug: true`

If you check out this branch https://github.com/Financial-Times/n-service-worker/tree/rhys/reduced-firefox-test, run `npm install && make -j2 test-chrome test-firefox` you should see the different results

(NOTE - the test bootstrapping is a little buggy in this branch, so if you see failures in both browsers try clearing idb and the cache api and unregistering the sw)
How do I build this on windows using just node/npm?  Or can you just package up the resulting dist directory and attach it here?
(Reporter)

Comment 5

3 years ago
The make task just calls karma - a node process - with some options, so you can try running `karma start --autoWatch=true --singleRun=false --browsers=Firefox` in your terminal. If that doesn't work I'll try saving the karma test page and uploading as a zip
Did you ever get this built locally, Ben? Did anything change in the meantime in our Response implementation?
Flags: needinfo?(bkelly)
Priority: -- → P2
I'm going to close this.  We were never able to reproduce.  Happy to investigate if the problem can be shown with a glitch.com example or something.
Status: UNCONFIRMED → RESOLVED
Last Resolved: a year ago
Flags: needinfo?(wheresrhys)
Flags: needinfo?(bkelly)
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.