Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: pass full response headers in net module #21770

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 15 additions & 6 deletions lib/browser/api/net.js
Expand Up @@ -53,18 +53,27 @@ class IncomingMessage extends Readable {

get headers () {
const filteredHeaders = {}
const rawHeaders = this._responseHead.headers
Object.keys(rawHeaders).forEach(header => {
if (header in filteredHeaders && discardableDuplicateHeaders.has(header)) {
const { rawHeaders } = this._responseHead
rawHeaders.forEach(header => {
if (Object.prototype.hasOwnProperty.call(filteredHeaders, header.key) &&
discardableDuplicateHeaders.has(header.key)) {
// do nothing with discardable duplicate headers
} else {
if (header === 'set-cookie') {
if (header.key === 'set-cookie') {
// keep set-cookie as an array per Node.js rules
// see https://nodejs.org/api/http.html#http_message_headers
filteredHeaders[header] = rawHeaders[header]
if (Object.prototype.hasOwnProperty.call(filteredHeaders, header.key)) {
filteredHeaders[header.key].push(header.value)
} else {
filteredHeaders[header.key] = [header.value]
}
} else {
// for non-cookie headers, the values are joined together with ', '
filteredHeaders[header] = rawHeaders[header].join(', ')
if (Object.prototype.hasOwnProperty.call(filteredHeaders, header.key)) {
filteredHeaders[header.key] += `, ${header.value}`
} else {
filteredHeaders[header.key] = header.value
}
}
}
})
Expand Down
38 changes: 31 additions & 7 deletions shell/browser/api/atom_api_url_loader.cc
Expand Up @@ -33,6 +33,28 @@
#include "shell/common/node_includes.h"
#include "shell/common/promise_util.h"

namespace mate {

template <>
struct Converter<std::pair<std::string, std::string>> {
static v8::Local<v8::Value> ToV8(
v8::Isolate* isolate,
const std::pair<std::string, std::string>& pair) {
mate::Dictionary dict = mate::Dictionary::CreateEmpty(isolate);
dict.Set("key", base::ToLowerASCII(pair.first));
dict.Set("value", pair.second);
return dict.GetHandle();
}
};

} // namespace mate

namespace electron {

namespace api {

namespace {

class BufferDataSource : public mojo::DataPipeProducer::DataSource {
public:
explicit BufferDataSource(base::span<char> buffer) {
Expand Down Expand Up @@ -200,12 +222,6 @@ class JSChunkedDataPipeGetter : public gin::Wrappable<JSChunkedDataPipeGetter>,
gin::WrapperInfo JSChunkedDataPipeGetter::kWrapperInfo = {
gin::kEmbedderNativeGin};

namespace electron {

namespace api {

namespace {

const net::NetworkTrafficAnnotationTag kTrafficAnnotation =
net::DefineNetworkTrafficAnnotation("electron_net_module", R"(
semantics {
Expand Down Expand Up @@ -343,6 +359,10 @@ mate::WrappableBase* SimpleURLLoaderWrapper::New(mate::Arguments* args) {
}
}

// Chromium filters headers using browser rules, while for net module we have
// every header passed.
request->report_raw_headers = true;

v8::Local<v8::Value> body;
v8::Local<v8::Value> chunk_pipe_getter;
if (opts.Get("body", &body)) {
Expand Down Expand Up @@ -419,8 +439,12 @@ void SimpleURLLoaderWrapper::OnResponseStarted(
mate::Dictionary dict = mate::Dictionary::CreateEmpty(isolate());
dict.Set("statusCode", response_head.headers->response_code());
dict.Set("statusMessage", response_head.headers->GetStatusText());
dict.Set("headers", response_head.headers.get());
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_request_response_info);
dict.Set("rawHeaders",
response_head.raw_request_response_info->response_headers);
Emit("response-started", final_url, dict);
}

Expand Down
17 changes: 17 additions & 0 deletions spec-main/api-net-spec.ts
Expand Up @@ -616,6 +616,23 @@ describe('net module', () => {
})
})

it('should be able to receive cookies', (done) => {
const cookie = ['cookie1', 'cookie2']
respondOnce.toSingleURL((request, response) => {
response.statusCode = 200
response.statusMessage = 'OK'
response.setHeader('set-cookie', cookie)
response.end()
}).then(serverUrl => {
const urlRequest = net.request(serverUrl)
urlRequest.on('response', (response) => {
expect(response.headers['set-cookie']).to.have.same.members(cookie)
done()
})
urlRequest.end()
})
})

it('should be able to abort an HTTP request before first write', async () => {
const serverUrl = await respondOnce.toSingleURL((request, response) => {
response.end()
Expand Down