walkero
/
nghttp2
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Don't sort headers in library code 

Remove sorting headers from library code. The application must sort
them if necessary. nghttpx and nghttpd do the sorting of the headers
in stable way if names are equal.
This commit is contained in:
Tatsuhiro Tsujikawa 2013-11-13 23:56:02 +09:00
parent 0ba2883940
commit 40347487c9
14 changed files with 152 additions and 91 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
lib/nghttp2_frame.c
View File

@ -621,20 +621,20 @@ void nghttp2_nv_array_del(nghttp2_nv *nva)
free(nva);
}
static int nghttp2_nv_name_compar(const void *lhs, const void *rhs)
static int bytes_compar(const uint8_t *a, size_t alen,
const uint8_t *b, size_t blen)
{
nghttp2_nv *a = (nghttp2_nv*)lhs, *b = (nghttp2_nv*)rhs;
if(a->namelen == b->namelen) {
return memcmp(a->name, b->name, a->namelen);
} else if(a->namelen < b->namelen) {
int rv = memcmp(a->name, b->name, a->namelen);
if(alen == blen) {
return memcmp(a, b, alen);
} else if(alen < blen) {
int rv = memcmp(a, b, alen);
if(rv == 0) {
return -1;
} else {
return rv;
}
} else {
int rv = memcmp(a->name, b->name, b->namelen);
int rv = memcmp(a, b, blen);
if(rv == 0) {
return 1;
} else {
@ -645,12 +645,24 @@ static int nghttp2_nv_name_compar(const void *lhs, const void *rhs)
int nghttp2_nv_compare_name(const nghttp2_nv *lhs, const nghttp2_nv *rhs)
{
return nghttp2_nv_name_compar(lhs, rhs);
return bytes_compar(lhs->name, lhs->namelen, rhs->name, rhs->namelen);
}
static int nv_compar(const void *lhs, const void *rhs)
{
const nghttp2_nv *a = (const nghttp2_nv*)lhs;
const nghttp2_nv *b = (const nghttp2_nv*)rhs;
int rv;
rv = bytes_compar(a->name, a->namelen, b->name, b->namelen);
if(rv == 0) {
return bytes_compar(a->value, a->valuelen, b->value, b->valuelen);
}
return rv;
}
void nghttp2_nv_array_sort(nghttp2_nv *nva, size_t nvlen)
{
qsort(nva, nvlen, sizeof(nghttp2_nv), nghttp2_nv_name_compar);
qsort(nva, nvlen, sizeof(nghttp2_nv), nv_compar);
}
ssize_t nghttp2_nv_array_from_cstr(nghttp2_nv **nva_ptr, const char **nv)
@ -694,7 +706,6 @@ ssize_t nghttp2_nv_array_from_cstr(nghttp2_nv **nva_ptr, const char **nv)
data += len;
++p;
}
nghttp2_nv_array_sort(*nva_ptr, nvlen);
return nvlen;
}
@ -737,7 +748,6 @@ ssize_t nghttp2_nv_array_copy(nghttp2_nv **nva_ptr,
data += nva[i].valuelen;
++p;
}
nghttp2_nv_array_sort(*nva_ptr, nvlen);
return nvlen;
}

4
lib/nghttp2_frame.h
View File

@ -523,8 +523,8 @@ nghttp2_settings_entry* nghttp2_frame_iv_copy(const nghttp2_settings_entry *iv,
int nghttp2_frame_nv_check_null(const char **nv);
/*
* Sorts the |nva| in ascending order of name. The relative order of
* the same name pair is undefined.
* Sorts the |nva| in ascending order of name and value. If names are
* equivalent, sort them by value.
*/
void nghttp2_nv_array_sort(nghttp2_nv *nva, size_t nvlen);

1
lib/nghttp2_hd.c
View File

@ -1352,7 +1352,6 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_context *inflater,
goto fail;
}
}
nghttp2_nv_array_sort(nva_out.nva, nva_out.nvlen);
*nva_ptr = nva_out.nva;
return nva_out.nvlen;
fail:

29
src/HttpServer.cc
View File

@ -707,13 +707,12 @@ void prepare_response(Request *req, Http2Handler *hd)
} // namespace
namespace {
void append_nv(Request *req, nghttp2_nv *nva, size_t nvlen)
void append_nv(Request *req, const std::vector<const nghttp2_nv*>& nva)
{
for(size_t i = 0; i < nvlen; ++i) {
req->headers.push_back({
std::string(nva[i].name, nva[i].name + nva[i].namelen),
std::string(nva[i].value, nva[i].value + nva[i].valuelen)
});
for(auto nv : nva) {
req->headers.push_back(std::make_pair
(std::string(nv->name, nv->name + nv->namelen),
std::string(nv->value, nv->value + nv->valuelen)));
}
}
} // namespace
@ -738,16 +737,14 @@ int hd_on_frame_recv_callback
switch(frame->headers.cat) {
case NGHTTP2_HCAT_REQUEST: {
int32_t stream_id = frame->hd.stream_id;
if(!http2::check_http2_headers(frame->headers.nva,
frame->headers.nvlen)) {
auto nva = http2::sort_nva(frame->headers.nva, frame->headers.nvlen);
if(!http2::check_http2_headers(nva)) {
nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, stream_id,
NGHTTP2_PROTOCOL_ERROR);
return 0;
}
for(size_t i = 0; REQUIRED_HEADERS[i]; ++i) {
if(!http2::get_unique_header(frame->headers.nva,
frame->headers.nvlen,
REQUIRED_HEADERS[i])) {
if(!http2::get_unique_header(nva, REQUIRED_HEADERS[i])) {
nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, stream_id,
NGHTTP2_PROTOCOL_ERROR);
return 0;
@ -756,18 +753,14 @@ int hd_on_frame_recv_callback
// intermediary translating from HTTP/1 request to HTTP/2 may
// not produce :authority header field. In this case, it should
// provide host HTTP/1.1 header field.
if(!http2::get_unique_header(frame->headers.nva,
frame->headers.nvlen,
":authority") &&
!http2::get_unique_header(frame->headers.nva,
frame->headers.nvlen,
"host")) {
if(!http2::get_unique_header(nva, ":authority") &&
!http2::get_unique_header(nva, "host")) {
nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, stream_id,
NGHTTP2_PROTOCOL_ERROR);
return 0;
}
auto req = util::make_unique<Request>(stream_id);
append_nv(req.get(), frame->headers.nva, frame->headers.nvlen);
append_nv(req.get(), nva);
hd->add_stream(stream_id, std::move(req));
break;
}

45
src/http2.cc
View File

@ -195,50 +195,69 @@ size_t HTTP1_IGN_HDLEN = sizeof(HTTP1_IGN_HD)/sizeof(HTTP1_IGN_HD[0]);
} // namespace
namespace {
auto nv_name_less = [](const nghttp2_nv& lhs, const nghttp2_nv& rhs)
auto nv_name_less = [](const nghttp2_nv *lhs, const nghttp2_nv *rhs)
{
return nghttp2_nv_compare_name(&lhs, &rhs) < 0;
return nghttp2_nv_compare_name(lhs, rhs) < 0;
};
} // namespace
bool check_http2_headers(const nghttp2_nv *nva, size_t nvlen)
bool check_http2_headers(const std::vector<const nghttp2_nv*>& nva)
{
for(size_t i = 0; i < DISALLOWED_HDLEN; ++i) {
nghttp2_nv nv = {(uint8_t*)DISALLOWED_HD[i], nullptr,
(uint16_t)strlen(DISALLOWED_HD[i]), 0};
if(std::binary_search(&nva[0], &nva[nvlen], nv, nv_name_less)) {
if(std::binary_search(std::begin(nva), std::end(nva), &nv, nv_name_less)) {
return false;
}
}
return true;
}
const nghttp2_nv* get_unique_header(const nghttp2_nv *nva, size_t nvlen,
std::vector<const nghttp2_nv*> sort_nva(const nghttp2_nv *nva, size_t nvlen)
{
auto res = std::vector<const nghttp2_nv*>();
res.reserve(nvlen);
for(size_t i = 0; i < nvlen; ++i) {
res.push_back(&nva[i]);
}
std::sort(std::begin(res), std::end(res),
[](const nghttp2_nv *lhs, const nghttp2_nv *rhs)
{
auto rv = nghttp2_nv_compare_name(lhs, rhs);
if(rv == 0) {
return lhs < rhs;
}
return rv < 0;
});
return res;
}
const nghttp2_nv* get_unique_header(const std::vector<const nghttp2_nv*>& nva,
const char *name)
{
size_t namelen = strlen(name);
nghttp2_nv nv = {(uint8_t*)name, nullptr, (uint16_t)namelen, 0};
auto i = std::lower_bound(&nva[0], &nva[nvlen], nv, nv_name_less);
if(i != &nva[nvlen] && util::streq(i->name, i->namelen,
auto i = std::lower_bound(std::begin(nva), std::end(nva), &nv, nv_name_less);
if(i != std::end(nva) && util::streq((*i)->name, (*i)->namelen,
(const uint8_t*)name, namelen)) {
auto j = i + 1;
if(j == &nva[nvlen] || !util::streq(j->name, j->namelen,
if(j == std::end(nva) || !util::streq((*j)->name, (*j)->namelen,
(const uint8_t*)name, namelen)) {
return i;
return *i;
}
}
return nullptr;
}
const nghttp2_nv* get_header(const nghttp2_nv *nva, size_t nvlen,
const nghttp2_nv* get_header(const std::vector<const nghttp2_nv*>& nva,
const char *name)
{
size_t namelen = strlen(name);
nghttp2_nv nv = {(uint8_t*)name, nullptr, (uint16_t)namelen, 0};
auto i = std::lower_bound(&nva[0], &nva[nvlen], nv, nv_name_less);
if(i != &nva[nvlen] && util::streq(i->name, i->namelen,
auto i = std::lower_bound(std::begin(nva), std::end(nva), &nv, nv_name_less);
if(i != std::end(nva) && util::streq((*i)->name, (*i)->namelen,
(const uint8_t*)name, namelen)) {
return i;
return *i;
}
return nullptr;
}

10
src/http2.h
View File

@ -67,18 +67,22 @@ bool check_http2_allowed_header(const char *name);
// Checks that headers |nva| including |nvlen| entries do not contain
// disallowed header fields in HTTP/2.0 spec. This function returns
// true if |nva| does not contains such headers.
bool check_http2_headers(const nghttp2_nv *nva, size_t nvlen);
bool check_http2_headers(const std::vector<const nghttp2_nv*>& nva);
// Returns sorted |nva| with |nvlen| elements. This sort is stable
// sort.
std::vector<const nghttp2_nv*> sort_nva(const nghttp2_nv *nva, size_t nvlen);
// Returns the pointer to the entry in |nva| which has name |name| and
// the |name| is uinque in the |nva|. If no such entry exist, returns
// nullptr.
const nghttp2_nv* get_unique_header(const nghttp2_nv *nva, size_t nvlen,
const nghttp2_nv* get_unique_header(const std::vector<const nghttp2_nv*>& nva,
const char *name);
// Returns the poiter to the entry in |nva| which has name |name|. If
// more than one entries which have the name |name|, first occurrence
// in |nva| is returned. If no such entry exist, returns nullptr.
const nghttp2_nv* get_header(const nghttp2_nv *nva, size_t nvlen,
const nghttp2_nv* get_header(const std::vector<const nghttp2_nv*>& nva,
const char *name);
// Returns std::string version of nv->name with nv->namelen bytes.

52
src/http2_test.cc
View File

@ -24,6 +24,7 @@
*/
#include "http2_test.h"
#include <cstring>
#include <iostream>
#include <CUnit/CUnit.h>
@ -38,22 +39,53 @@ using namespace nghttp2;
namespace shrpx {
namespace {
template<typename cstr1, typename cstr2>
int bstrcmp(cstr1 *a, cstr2 *b)
{
return strcmp((const char*)a, (const char*)b);
}
} // namespace
void test_http2_sort_nva(void)
{
nghttp2_nv nv[] = {MAKE_NV("alpha", "1"),
MAKE_NV("bravo", "9"),
MAKE_NV("bravo", "8"),
MAKE_NV("charlie", "5"),
MAKE_NV("bravo", "3"),
MAKE_NV("bravo", "4")};
auto nvlen = sizeof(nv)/sizeof(nv[0]);
auto nva = http2::sort_nva(nv, nvlen);
CU_ASSERT(nvlen == nva.size());
CU_ASSERT(0 == bstrcmp("alpha", nva[0]->name));
CU_ASSERT(0 == bstrcmp("bravo", nva[1]->name));
CU_ASSERT(0 == bstrcmp("9", nva[1]->value));
CU_ASSERT(0 == bstrcmp("bravo", nva[2]->name));
CU_ASSERT(0 == bstrcmp("8", nva[2]->value));
CU_ASSERT(0 == bstrcmp("bravo", nva[3]->name));
CU_ASSERT(0 == bstrcmp("3", nva[3]->value));
CU_ASSERT(0 == bstrcmp("bravo", nva[4]->name));
CU_ASSERT(0 == bstrcmp("4", nva[4]->value));
CU_ASSERT(0 == bstrcmp("charlie", nva[5]->name));
}
void test_http2_check_http2_headers(void)
{
nghttp2_nv nv1[] = {MAKE_NV("alpha", "1"),
MAKE_NV("bravo", "2"),
MAKE_NV("upgrade", "http2")};
CU_ASSERT(!http2::check_http2_headers(nv1, 3));
CU_ASSERT(!http2::check_http2_headers(http2::sort_nva(nv1, 3)));
nghttp2_nv nv2[] = {MAKE_NV("connection", "1"),
MAKE_NV("delta", "2"),
MAKE_NV("echo", "3")};
CU_ASSERT(!http2::check_http2_headers(nv2, 3));
CU_ASSERT(!http2::check_http2_headers(http2::sort_nva(nv2, 3)));
nghttp2_nv nv3[] = {MAKE_NV("alpha", "1"),
MAKE_NV("bravo", "2"),
MAKE_NV("te2", "3")};
CU_ASSERT(http2::check_http2_headers(nv3, 3));
CU_ASSERT(http2::check_http2_headers(http2::sort_nva(nv3, 3)));
}
void test_http2_get_unique_header(void)
@ -65,15 +97,16 @@ void test_http2_get_unique_header(void)
MAKE_NV("delta", "5"),
MAKE_NV("echo", "6"),};
size_t nvlen = sizeof(nv)/sizeof(nv[0]);
auto nva = http2::sort_nva(nv, nvlen);
const nghttp2_nv *rv;
rv = http2::get_unique_header(nv, nvlen, "delta");
rv = http2::get_unique_header(nva, "delta");
CU_ASSERT(rv != nullptr);
CU_ASSERT(util::streq("delta", rv->name, rv->namelen));
rv = http2::get_unique_header(nv, nvlen, "bravo");
rv = http2::get_unique_header(nva, "bravo");
CU_ASSERT(rv == nullptr);
rv = http2::get_unique_header(nv, nvlen, "foxtrot");
rv = http2::get_unique_header(nva, "foxtrot");
CU_ASSERT(rv == nullptr);
}
@ -86,16 +119,17 @@ void test_http2_get_header(void)
MAKE_NV("delta", "5"),
MAKE_NV("echo", "6"),};
size_t nvlen = sizeof(nv)/sizeof(nv[0]);
auto nva = http2::sort_nva(nv, nvlen);
const nghttp2_nv *rv;
rv = http2::get_header(nv, nvlen, "delta");
rv = http2::get_header(nva, "delta");
CU_ASSERT(rv != nullptr);
CU_ASSERT(util::streq("delta", rv->name, rv->namelen));
rv = http2::get_header(nv, nvlen, "bravo");
rv = http2::get_header(nva, "bravo");
CU_ASSERT(rv != nullptr);
CU_ASSERT(util::streq("bravo", rv->name, rv->namelen));
rv = http2::get_header(nv, nvlen, "foxtrot");
rv = http2::get_header(nva, "foxtrot");
CU_ASSERT(rv == nullptr);
}

1
src/http2_test.h
View File

@ -27,6 +27,7 @@
namespace shrpx {
void test_http2_sort_nva(void);
void test_http2_check_http2_headers(void);
void test_http2_get_unique_header(void);
void test_http2_get_header(void);

1
src/shrpx-unittest.cc
View File

@ -69,6 +69,7 @@ int main(int argc, char* argv[])
shrpx::test_shrpx_ssl_create_lookup_tree) ||
!CU_add_test(pSuite, "ssl_cert_lookup_tree_add_cert_from_file",
shrpx::test_shrpx_ssl_cert_lookup_tree_add_cert_from_file) ||
!CU_add_test(pSuite, "http2_sort_nva", shrpx::test_http2_sort_nva) ||
!CU_add_test(pSuite, "http2_check_http2_headers",
shrpx::test_http2_check_http2_headers) ||
!CU_add_test(pSuite, "http2_get_unique_header",

2
src/shrpx_downstream.cc
View File

@ -156,7 +156,7 @@ void normalize_headers(Headers& headers)
for(auto& kv : headers) {
util::inp_strlower(kv.first);
}
std::sort(std::begin(headers), std::end(headers), name_less);
std::stable_sort(std::begin(headers), std::end(headers), name_less);
}
} // namespace

25
src/shrpx_http2_session.cc
View File

@ -820,11 +820,10 @@ int on_frame_recv_callback
NGHTTP2_INTERNAL_ERROR);
break;
}
auto nva = frame->headers.nva;
auto nvlen = frame->headers.nvlen;
// nva is no longer sorted
auto nva = http2::sort_nva(frame->headers.nva, frame->headers.nvlen);
// Assuming that nva is sorted by name.
if(!http2::check_http2_headers(nva, nvlen)) {
if(!http2::check_http2_headers(nva)) {
http2session->submit_rst_stream(frame->hd.stream_id,
NGHTTP2_PROTOCOL_ERROR);
downstream->set_response_state(Downstream::MSG_RESET);
@ -832,14 +831,14 @@ int on_frame_recv_callback
return 0;
}
for(size_t i = 0; i < nvlen; ++i) {
if(nva[i].namelen > 0 && nva[i].name[0] != ':') {
downstream->add_response_header(http2::name_to_str(&nva[i]),
http2::value_to_str(&nva[i]));
for(auto nv : nva) {
if(nv->namelen > 0 && nv->name[0] != ':') {
downstream->add_response_header(http2::name_to_str(nv),
http2::value_to_str(nv));
}
}
auto status = http2::get_unique_header(nva, nvlen, ":status");
auto status = http2::get_unique_header(nva, ":status");
if(!status || http2::value_lws(status)) {
http2session->submit_rst_stream(frame->hd.stream_id,
NGHTTP2_PROTOCOL_ERROR);
@ -855,7 +854,7 @@ int on_frame_recv_callback
downstream->set_response_major(1);
downstream->set_response_minor(1);
auto content_length = http2::get_header(nva, nvlen, "content-length");
auto content_length = http2::get_header(nva, "content-length");
if(!content_length && downstream->get_request_method() != "HEAD" &&
downstream->get_request_method() != "CONNECT") {
unsigned int status;
@ -879,11 +878,11 @@ int on_frame_recv_callback
if(LOG_ENABLED(INFO)) {
std::stringstream ss;
for(size_t i = 0; i < frame->headers.nvlen; ++i) {
for(auto nv : nva) {
ss << TTY_HTTP_HD;
ss.write(reinterpret_cast<char*>(nva[i].name), nva[i].namelen);
ss.write(reinterpret_cast<char*>(nv->name), nv->namelen);
ss << TTY_RST << ": ";
ss.write(reinterpret_cast<char*>(nva[i].value), nva[i].valuelen);
ss.write(reinterpret_cast<char*>(nv->value), nv->valuelen);
ss << "\n";
}
SSLOG(INFO, http2session) << "HTTP response headers. stream_id="

33
src/shrpx_http2_upstream.cc
View File

@ -237,16 +237,16 @@ int on_frame_recv_callback
upstream->add_downstream(downstream);
downstream->init_response_body_buf();
auto nva = frame->headers.nva;
auto nvlen = frame->headers.nvlen;
// nva is no longer sorted
auto nva = http2::sort_nva(frame->headers.nva, frame->headers.nvlen);
if(LOG_ENABLED(INFO)) {
std::stringstream ss;
for(size_t i = 0; i < frame->headers.nvlen; ++i) {
for(auto nv : nva) {
ss << TTY_HTTP_HD;
ss.write(reinterpret_cast<char*>(nva[i].name), nva[i].namelen);
ss.write(reinterpret_cast<char*>(nv->name), nv->namelen);
ss << TTY_RST << ": ";
ss.write(reinterpret_cast<char*>(nva[i].value), nva[i].valuelen);
ss.write(reinterpret_cast<char*>(nv->value), nv->valuelen);
ss << "\n";
}
ULOG(INFO, upstream) << "HTTP request headers. stream_id="
@ -254,24 +254,23 @@ int on_frame_recv_callback
<< "\n" << ss.str();
}
// Assuming that nva is sorted by name.
if(!http2::check_http2_headers(nva, nvlen)) {
if(!http2::check_http2_headers(nva)) {
upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR);
return 0;
}
for(size_t i = 0; i < nvlen; ++i) {
if(nva[i].namelen > 0 && nva[i].name[0] != ':') {
downstream->add_request_header(http2::name_to_str(&nva[i]),
http2::value_to_str(&nva[i]));
for(auto nv : nva) {
if(nv->namelen > 0 && nv->name[0] != ':') {
downstream->add_request_header(http2::name_to_str(nv),
http2::value_to_str(nv));
}
}
auto host = http2::get_unique_header(nva, nvlen, "host");
auto authority = http2::get_unique_header(nva, nvlen, ":authority");
auto path = http2::get_unique_header(nva, nvlen, ":path");
auto method = http2::get_unique_header(nva, nvlen, ":method");
auto scheme = http2::get_unique_header(nva, nvlen, ":scheme");
auto host = http2::get_unique_header(nva, "host");
auto authority = http2::get_unique_header(nva, ":authority");
auto path = http2::get_unique_header(nva, ":path");
auto method = http2::get_unique_header(nva, ":method");
auto scheme = http2::get_unique_header(nva, ":scheme");
bool is_connect = method &&
util::streq("CONNECT", method->value, method->valuelen);
bool having_host = http2::non_empty_value(host);
@ -297,7 +296,7 @@ int on_frame_recv_callback
}
if(!is_connect &&
(frame->hd.flags & NGHTTP2_FLAG_END_STREAM) == 0) {
auto content_length = http2::get_header(nva, nvlen, "content-length");
auto content_length = http2::get_header(nva, "content-length");
if(!content_length || http2::value_lws(content_length)) {
// If content-length is missing,
// Downstream::push_upload_data_chunk will fail and

1
tests/nghttp2_frame_test.c
View File

@ -118,6 +118,7 @@ void test_nghttp2_frame_pack_headers()
NGHTTP2_FLAG_PRIORITY,
1000000007, &oframe.hd);
CU_ASSERT(1 << 20 == oframe.pri);
nghttp2_nv_array_sort(oframe.nva, oframe.nvlen);
CU_ASSERT(nvnameeq("method", &oframe.nva[0]));
free(buf);

1
tests/nghttp2_hd_test.c
View File

@ -38,6 +38,7 @@
static void assert_nv_equal(nghttp2_nv *a, nghttp2_nv *b, size_t len)
{
size_t i;
nghttp2_nv_array_sort(b, len);
for(i = 0; i < len; ++i, ++a, ++b) {
CU_ASSERT(nghttp2_nv_equal(a, b));
}