Browse Source

fix: incorrect Content-Disposition encoding (#25961)

* Fixed the filename in content-disposition chaos.

* Fixed lint fail.

* Add test code and some comment.

* Fix lint error.

* fix lint error.

* * test: onHeadersReceived doesn't change the filename in "content-disposition" (#25628)

* fix lint error.
bigben0123 4 years ago
parent
commit
84a42a050e
2 changed files with 29 additions and 0 deletions
  1. 15 0
      shell/browser/api/electron_api_web_request.cc
  2. 14 0
      spec-main/api-web-request-spec.ts

+ 15 - 0
shell/browser/api/electron_api_web_request.cc

@@ -13,6 +13,7 @@
 #include "gin/converter.h"
 #include "gin/dictionary.h"
 #include "gin/object_template_builder.h"
+#include "net/http/http_content_disposition.h"
 #include "shell/browser/api/electron_api_session.h"
 #include "shell/browser/api/electron_api_web_contents.h"
 #include "shell/browser/electron_browser_context.h"
@@ -115,6 +116,20 @@ v8::Local<v8::Value> HttpResponseHeadersToV8(
     std::string value;
     while (headers->EnumerateHeaderLines(&iter, &key, &value)) {
       base::Value* values = response_headers.FindListKey(key);
+      // Note that Web servers not developed with nodejs allow non-utf8
+      // characters in content-disposition's filename field. Use Chromium's
+      // HttpContentDisposition class to decode the correct encoding instead of
+      // arbitrarily converting it to UTF8. It should also be noted that if the
+      // encoding is not specified, HttpContentDisposition will transcode
+      // according to the system's encoding.
+      if (base::EqualsCaseInsensitiveASCII("Content-Disposition", key) &&
+          !value.empty()) {
+        net::HttpContentDisposition header(value, std::string());
+        std::string decodedFilename =
+            header.is_attachment() ? " attachement" : " inline";
+        decodedFilename += "; filename=" + header.filename();
+        value = decodedFilename;
+      }
       if (!values)
         values = response_headers.SetKey(key, base::ListValue());
       values->Append(value);

+ 14 - 0
spec-main/api-web-request-spec.ts

@@ -17,6 +17,10 @@ describe('webRequest module', () => {
       res.statusCode = 301;
       res.setHeader('Location', 'http://' + req.rawHeaders[1]);
       res.end();
+    } else if (req.url === '/contentDisposition') {
+      res.setHeader('content-disposition', [' attachement; filename=aa%E4%B8%ADaa.txt']);
+      const content = req.url;
+      res.end(content);
     } else {
       res.setHeader('Custom', ['Header']);
       let content = req.url;
@@ -297,6 +301,16 @@ describe('webRequest module', () => {
       expect(data).to.equal('/');
     });
 
+    it('does not change content-disposition header by default', async () => {
+      ses.webRequest.onHeadersReceived((details, callback) => {
+        expect(details.responseHeaders!['content-disposition']).to.deep.equal([' attachement; filename=aa中aa.txt']);
+        callback({});
+      });
+      const { data, headers } = await ajax(defaultURL + 'contentDisposition');
+      expect(headers).to.match(/^content-disposition: attachement; filename=aa%E4%B8%ADaa.txt$/m);
+      expect(data).to.equal('/contentDisposition');
+    });
+
     it('follows server redirect', async () => {
       ses.webRequest.onHeadersReceived((details, callback) => {
         const responseHeaders = details.responseHeaders;