Skip to content
Permalink
Browse files

fix: allow stream protocols to return headers with multiple values (#…

…14887) (#18094)

* fix: allow stream protocols to return headers with multiple values

This allows stream protocols to return headers with multiple values as
an array of values.

Fixes #14778

* Prefer ConvertFromV8

* Cleanup header conversion

1. Deduplicate the code by using a lambda
2. Remove duplicate calls to headers->Get(key)

* Fix broken test

Headers with multiple values are now being converted correctly, this
test asserted the wrong behavior.
  • Loading branch information...
miniak authored and MarshallOfSound committed May 1, 2019
1 parent e5e94fc commit ac714a1d022be47209fbc4a0b553f50e0e0d9f34
Showing with 63 additions and 12 deletions.
  1. +32 −11 atom/common/native_mate_converters/net_converter.cc
  2. +31 −1 spec/api-protocol-spec.js
@@ -181,24 +181,45 @@ bool Converter<net::HttpResponseHeaders*>::FromV8(
if (!val->IsObject()) {
return false;
}

auto addHeaderFromValue = [&isolate, &out](
const std::string& key,
const v8::Local<v8::Value>& localVal) {
auto context = isolate->GetCurrentContext();
v8::Local<v8::String> localStrVal;
if (!localVal->ToString(context).ToLocal(&localStrVal)) {
return false;
}
std::string value;
mate::ConvertFromV8(isolate, localStrVal, &value);
out->AddHeader(key + ": " + value);
return true;
};

auto context = isolate->GetCurrentContext();
auto headers = v8::Local<v8::Object>::Cast(val);
auto keys = headers->GetOwnPropertyNames();
for (uint32_t i = 0; i < keys->Length(); i++) {
v8::Local<v8::String> key, value;
if (!keys->Get(i)->ToString(context).ToLocal(&key)) {
v8::Local<v8::String> keyVal;
if (!keys->Get(i)->ToString(context).ToLocal(&keyVal)) {
return false;
}
if (!headers->Get(key)->ToString(context).ToLocal(&value)) {
return false;
std::string key;
mate::ConvertFromV8(isolate, keyVal, &key);

auto localVal = headers->Get(keyVal);
if (localVal->IsArray()) {
auto values = v8::Local<v8::Array>::Cast(localVal);
for (uint32_t j = 0; j < values->Length(); j++) {
if (!addHeaderFromValue(key, values->Get(j))) {
return false;
}
}
} else {
if (!addHeaderFromValue(key, localVal)) {
return false;
}
}
v8::String::Utf8Value key_utf8(key);
v8::String::Utf8Value value_utf8(value);
std::string k(*key_utf8, key_utf8.length());
std::string v(*value_utf8, value_utf8.length());
std::ostringstream tmp;
tmp << k << ": " << v;
out->AddHeader(tmp.str());
}
return true;
}
@@ -542,7 +542,7 @@ describe('protocol module', () => {
cache: false,
success: (data, _, request) => {
assert.strictEqual(request.status, 200)
assert.strictEqual(request.getResponseHeader('x-electron'), 'a,b')
assert.strictEqual(request.getResponseHeader('x-electron'), 'a, b')
assert.strictEqual(data, text)
done()
},
@@ -626,6 +626,36 @@ describe('protocol module', () => {
})
assert.strictEqual(r.length, data.length)
})

it('returns response multiple response headers with the same name', (done) => {
const handler = (request, callback) => {
callback({
headers: {
'header1': ['value1', 'value2'],
'header2': 'value3'
},
data: getStream()
})
}

protocol.registerStreamProtocol(protocolName, handler, (error) => {
if (error) return done(error)
$.ajax({
url: protocolName + '://fake-host',
cache: false,
success: (data, status, request) => {
// SUBTLE: when the response headers have multiple values it
// separates values by ", ". When the response headers are incorrectly
// converting an array to a string it separates values by ",".
assert.strictEqual(request.getAllResponseHeaders(), 'header1: value1, value2\r\nheader2: value3\r\n')
done()
},
error: (xhr, errorType, error) => {
done(error || new Error(`Request failed: ${xhr.status}`))
}
})
})
})
})

describe('protocol.isProtocolHandled', () => {

0 comments on commit ac714a1

Please sign in to comment.
You can’t perform that action at this time.