Repository: electron/electron (language: cpp)

Issue:
net.request response is missing content-type header in 304 responses
### Preflight Checklist

* [x ] I have read the [Contributing Guidelines](https://github.com/electron/electron/blob/master/CONTRIBUTING.md) for this project.
* [ x] I agree to follow the [Code of Conduct](https://github.com/electron/electron/blob/master/CODE_OF_CONDUCT.md) that this project adheres to.
* [ x] I have searched the issue tracker for an issue that matches the one I want to file, without success.

### Issue Details

* **Electron Version:**
10.1.7
* **Operating System:**
Windows 10 Enterprise 1909

When making a request with `net.request`, I can see that my response has a `content-type` header of `application/json; charset=utf-8` inside of my `webRequest.onHeadersReceived` handler:

```
electron.session.defaultSession.webRequest.onHeadersReceived((details, callback) => {
  // details.responseHeaders['content-type'] === 'application/json; charset=utf-8'
});
```

But by the time the `net.request` `request.on('response')` handler receives the headers, the content-type is gone.

```
request.on('response', (response) => {
  /* 
  response.headers === {
    // no content-type
  };
  */
});
```

I haven't tested if this is an issue in all types of responses, but I can tell you this is occurring without fail for the case when the `response.statusCode === 200`, but the `response.headers.status === 304`.

### Expected Behavior
For `net.request` response handler to receive the content-type that clearly should exist on the response (proved by `webRequest.onHeadersReceived`).

### Actual Behavior
`net.request` response handler is receiving what seems to be a cached set of headers (or something?), which do not match what is seen in `webRequest.onHeadersReceived`.



Discussion / hints:
Thanks for reporting this and helping to make Electron better!

Would it be possible for you to make a standalone testcase with only the code necessary to reproduce the issue? For example, [Electron Fiddle](https://www.electronjs.org/fiddle) is a great tool for making small test cases and makes it easy to publish your test case to a [gist](https://gist.github.com) that Electron maintainers can use.

Stand-alone test cases make fixing issues go more smoothly: it ensure everyone's looking at the same issue, it removes all unnecessary variables from the equation, and it can also provide the basis for automated regression tests.

I'm adding the `blocked/needs-repro` label for this reason. After you make a test case, please link to it in a followup comment.

Thanks in advance! Your help is appreciated.

No repro example yet, but just to add some details we've found since: `net` seems to be adding the `status` header on its own. We've mostly (but not exclusively) been testing against GitHub's REST API, and we've confirmed through Postman that the servers we've hit don't include the `status` header, but it consistently comes back `304` on many requests, unless we obliterate the `Electron/Cache` folder from the user data. So far it seems like we're getting back legitimate 200s and net is arbitrarily adding `status: 304` and deleting the `content-type` header (and removing custom headers,  #27894 which may be the same issue). When we force GitHub to give us a 304 by including an ETag in `If-None-Match`, `net.request` gives us back a completely normal-looking 304 with no body. Basically we're just really perplexed by the inconsistent and arbitrary way that `net` sort-of-does-sort-of-doesn't transparently populate 304 results from the Electron cache, or just decides not to even attempt such a population at all. We haven't been able to find any existing issues or documentation that speak to its intended behavior.
I just hit the same problem : electron does not repopulate `content-type` and `content-length` when a 304 is "transformed" in 200 from the cache.

Also, I can confirm that the behaviour was different on electron <= 6 : up to electron6, the 200 seen in this case correctly has `content-type` and `content-length`.
Here is a standalone testcase : 

https://gist.github.com/arantes555/99f8976266d340bad42b2894cb50d7d5

This shows that when the server responds with a 200, `response` correctly contains `content-type` and `content-length`. But when the server responds with a 304, `response` is shown to be a 200 but does **not** contain  `content-type` and `content-length`.

I also showed the behaviour of `onHeadersReceived` that @themadtitanmathos noticed : on this callback, headers are correct. 

Also, with electron@6, everything is correct.
we never got to the point of setting up our own server to test, but testing against GitHub and GitLab APIs, we only got a repro _while we had something in the cache._ For whatever reason, when we manually set our own `If-None-Match` that would prompt a 304 from the server, we got back an unmolested 304 (i.e. 304 status code, no body). We came to the conclusion that it was because of a bad interaction with the Electron and/or Chrome request caches. I'm confident that if you cleared both caches and ran the 304 first, you would get back the `content-type` and `content-length` as expected. In other words, I believe it's some problematic way that the cache results are being "merged" into the final results that causes the headers to drop. The same mechanism is probably also preventing the `onHeadersReceived` callback from working (#27894) by overriding the results _after_ the callback occurs.
Thanks for the standalone repro @arantes555! With that I've been able to confirm that this also happens on macOS, and in 12.x and 13.x.
This looks like a regression caused by https://github.com/electron/electron/pull/21552. As an experiment, I tried adding the non-"raw" headers to the internal response object:

```diff
diff --git a/shell/browser/api/electron_api_url_loader.cc b/shell/browser/api/electron_api_url_loader.cc
index afe48e40c..6a836f7af 100644
--- a/shell/browser/api/electron_api_url_loader.cc
+++ b/shell/browser/api/electron_api_url_loader.cc
@@ -68,6 +68,22 @@ struct Converter<network::mojom::CredentialsMode> {
   }
 };  // namespace gin

+template <>
+struct Converter<net::HttpResponseHeaders> {
+  static v8::Local<v8::Value> ToV8(
+      v8::Isolate* isolate,
+      const net::HttpResponseHeaders& headers) {
+    gin_helper::Dictionary dict = gin::Dictionary::CreateEmpty(isolate);
+    size_t i = 0;
+    std::string name;
+    std::string value;
+    while (headers.EnumerateHeaderLines(&i, &name, &value)) {
+      dict.Set(name, value);
+    }
+    return dict.GetHandle();
+  }
+};
+
 }  // namespace gin

 namespace electron {
@@ -561,6 +577,8 @@ void SimpleURLLoaderWrapper::OnResponseStarted(
   DCHECK(response_head.raw_request_response_info);
   dict.Set("rawHeaders",
            response_head.raw_request_response_info->response_headers);
+  dict.Set("headers",
+           *response_head.headers);
   Emit("response-started", final_url, dict);
 }
 
```
(NB. above patch is hacky and doesn't allow for repeated headers, it was just enough to let me see whether this theory about raw headers was correct.)

The `headers` object contained the correct `content-type` and `content-length`, even when the `rawHeaders` didn't. For example:

![image](https://user-images.githubusercontent.com/172800/110839757-c5ab4180-8258-11eb-87c9-91d4e4800524.png)

As further confirmation, the above PR was backported to 7.x in v7.1.10. When I run the fiddle on v7.1.9, it works as expected.

cc @zcbenz 
Comparing the `response.headers` between 6 and 7, it seems that we should pass filtered headers instead of the raw headers to JS. So https://github.com/electron/electron/pull/21552 should be reverted, but we would then need a fix to https://github.com/electron/electron/issues/20631.
We're not exactly sure why (this was reported from end users), but in very rare cases it's clear that this is also happening on non-304 responses as well, and no extra `status` header is added in those cases.

Right now our solution is to play fast and loose with content-type and simply attempt to parse anything missing its content-type as a JSON and see if it works. Hopefully nobody responds with plain-text `null`/`true` while the content-type is being dropped 🤞
I can confirm this is still happening up to 16.0.0-beta.2 .

Also, here is a simplified test-case : https://gist.github.com/arantes555/4849273fe59b87313f0cb472ae65ae5b
Thanks for checking, @arantes555.

For our part, despite our best efforts to work around this problem, we've continued to hear about mysterious proxy issues with users (which may be this exact issue, or an unrelated problem with `net.request`). We're planning to abandon `net.request` and try to do all our HTTP(S) requests strictly from renderers using more field-tested front-end browser APIs, and IPC to pipe data as necessary whenever the main process wants something from the internet. 🤷 
@Mr-Wallet Honestly, that is not a bad way to proceed ^^ the `net` library sometimes feels a little, let's say not a top priority for electron
Yeah, not to throw shade on such an amazing free (!) project... it seems to me that if other Electron apps were trying to use `net` to the degree that my team is, there would be more open issues and _way_ more noise on them.

Certainly, the limited attention available for Electron is going to mostly go with the majority of use cases, and most apps seem to be single-renderer and do absolutely everything from inside that single renderer. We're hoping that by acting more like a "normal" Electron app with regards to networking, we'll sidestep these issues entirely.
This is still an issue in 18.2.2 (and 19.0.0-beta.5). Any news or plans to fix this?
I have encountered the same issue while working with GitHub API.
`Content-type` header and some GitHub custom headers are missing if a response is returned from the cache.
Is there any workaround except disabling caching using the` 'Cache-Control': 'no-cache'` header?
This issue has been automatically marked as stale. **If this issue is still affecting you, please leave any comment** (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the [latest version of Electron](https://www.electronjs.org/releases/stable) or in the [beta](https://www.electronjs.org/releases/beta)—please include it with your comment!
bump
Any news ?
PS: `details` also does not return the list of headers that are supposed to be used by the request. This seems to be a precedence error. `onBeforeSendHeaders` should be triggered after the "default" headers have been populated.
Right, seems like the problem is that we're using `rawHeaders` instead of `headers` to fill the header list, but the content-type / content-length only come back in `headers`. Seems we might have to merge these somehow in order to fix this but preserve the fix for https://github.com/electron/electron/issues/20631.

Fix patch:
diff --git a/lib/browser/api/net.ts b/lib/browser/api/net.ts
index 6438eb0230c16..31845c7daf2db 100644
--- a/lib/browser/api/net.ts
+++ b/lib/browser/api/net.ts
@@ -56,31 +56,14 @@ class IncomingMessage extends Readable {
 
   get headers () {
     const filteredHeaders: Record<string, string | string[]> = {};
-    const { rawHeaders } = this._responseHead;
-    rawHeaders.forEach(header => {
-      const keyLowerCase = header.key.toLowerCase();
-      if (Object.prototype.hasOwnProperty.call(filteredHeaders, keyLowerCase) &&
-          discardableDuplicateHeaders.has(keyLowerCase)) {
-        // do nothing with discardable duplicate headers
-      } else {
-        if (keyLowerCase === 'set-cookie') {
-          // keep set-cookie as an array per Node.js rules
-          // see https://nodejs.org/api/http.html#http_message_headers
-          if (Object.prototype.hasOwnProperty.call(filteredHeaders, keyLowerCase)) {
-            (filteredHeaders[keyLowerCase] as string[]).push(header.value);
-          } else {
-            filteredHeaders[keyLowerCase] = [header.value];
-          }
-        } else {
-          // for non-cookie headers, the values are joined together with ', '
-          if (Object.prototype.hasOwnProperty.call(filteredHeaders, keyLowerCase)) {
-            filteredHeaders[keyLowerCase] += `, ${header.value}`;
-          } else {
-            filteredHeaders[keyLowerCase] = header.value;
-          }
-        }
-      }
-    });
+    const { headers, rawHeaders } = this._responseHead;
+    for (const [name, values] of Object.entries(headers)) {
+      filteredHeaders[name] = discardableDuplicateHeaders.has(name) ? values[0] : values.join(', ');
+    }
+    const cookies = rawHeaders.filter(({ key }) => key.toLowerCase() === 'set-cookie').map(({ value }) => value);
+    // keep set-cookie as an array per Node.js rules
+    // see https://nodejs.org/api/http.html#http_message_headers
+    if (cookies.length) { filteredHeaders['set-cookie'] = cookies; }
     return filteredHeaders;
   }
 
diff --git a/shell/browser/api/electron_api_url_loader.cc b/shell/browser/api/electron_api_url_loader.cc
index 3a2d329738966..6fed3bf105f7c 100644
--- a/shell/browser/api/electron_api_url_loader.cc
+++ b/shell/browser/api/electron_api_url_loader.cc
@@ -577,9 +577,7 @@ void SimpleURLLoaderWrapper::OnResponseStarted(
   dict.Set("statusCode", response_head.headers->response_code());
   dict.Set("statusMessage", response_head.headers->GetStatusText());
   dict.Set("httpVersion", response_head.headers->GetHttpVersion());
-  // Note that |response_head.headers| are filtered by Chromium and should not
-  // be used here.
-  DCHECK(!response_head.raw_response_headers.empty());
+  dict.Set("headers", response_head.headers.get());
   dict.Set("rawHeaders", response_head.raw_response_headers);
   Emit("response-started", final_url, dict);
 }
diff --git a/spec/api-net-spec.ts b/spec/api-net-spec.ts
index 97f7e5ca90c44..7e23afe2e8a18 100644
--- a/spec/api-net-spec.ts
+++ b/spec/api-net-spec.ts
@@ -623,6 +623,19 @@ describe('net module', () => {
       expect(response.headers['set-cookie']).to.have.same.members(cookie);
     });
 
+    it('should be able to receive content-type', async () => {
+      const contentType = 'mime/test; charset=test';
+      const serverUrl = await respondOnce.toSingleURL((request, response) => {
+        response.statusCode = 200;
+        response.statusMessage = 'OK';
+        response.setHeader('content-type', contentType);
+        response.end();
+      });
+      const urlRequest = net.request(serverUrl);
+      const response = await getResponse(urlRequest);
+      expect(response.headers['content-type']).to.equal(contentType);
+    });
+
     it('should not use the sessions cookie store by default', async () => {
       const serverUrl = await respondOnce.toSingleURL((request, response) => {
         response.statusCode = 200;
diff --git a/typings/internal-ambient.d.ts b/typings/internal-ambient.d.ts
index d972d03eda129..580fec7688b34 100644
--- a/typings/internal-ambient.d.ts
+++ b/typings/internal-ambient.d.ts
@@ -145,6 +145,7 @@ declare namespace NodeJS {
     statusMessage: string;
     httpVersion: { major: number, minor: number };
     rawHeaders: { key: string, value: string }[];
+    headers: Record<string, string[]>;
   };
 
   type RedirectInfo = {


Test patch: