Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Bug 742174 - Allow empty Location header. r=mcmanus, a=akeybl

  • Loading branch information...
commit c7d5f4eee17500f360d1556c6e0c39b3856e3406 1 parent e58ab00
jduell jduell authored
3  netwerk/protocol/http/nsHttpHeaderArray.cpp
@@ -89,9 +89,6 @@ nsHttpHeaderArray::SetHeaderFromNet(nsHttpAtom header, const nsACString &value)
89 89
90 90 if (!entry) {
91 91 if (value.IsEmpty()) {
92   - if (HeaderMustHaveValue(header)) {
93   - return NS_ERROR_CORRUPTED_CONTENT;
94   - }
95 92 if (!TrackEmptyHeader(header)) {
96 93 LOG(("Ignoring Empty Header: %s\n", header.get()));
97 94 return NS_OK; // ignore empty headers by default
11 netwerk/protocol/http/nsHttpHeaderArray.h
@@ -113,8 +113,6 @@ class nsHttpHeaderArray
113 113
114 114 // Header cannot be merged: only one value possible
115 115 bool IsSingletonHeader(nsHttpAtom header);
116   - // For some headers, we treat no value as possible CRLF attack
117   - bool HeaderMustHaveValue(nsHttpAtom header);
118 116 // For some headers we want to track empty values to prevent them being
119 117 // combined with non-empty ones as a CRLF attack vector
120 118 bool TrackEmptyHeader(nsHttpAtom header);
@@ -162,15 +160,10 @@ nsHttpHeaderArray::IsSingletonHeader(nsHttpAtom header)
162 160 }
163 161
164 162 inline bool
165   -nsHttpHeaderArray::HeaderMustHaveValue(nsHttpAtom header)
166   -{
167   - return header == nsHttp::Location;
168   -}
169   -
170   -inline bool
171 163 nsHttpHeaderArray::TrackEmptyHeader(nsHttpAtom header)
172 164 {
173   - return header == nsHttp::Content_Length;
  165 + return header == nsHttp::Content_Length ||
  166 + header == nsHttp::Location;
174 167 }
175 168
176 169 inline void
36 netwerk/test/unit/test_duplicate_headers.js
@@ -361,14 +361,13 @@ function completeTest11(request, data, ctx)
361 361 }
362 362
363 363 ////////////////////////////////////////////////////////////////////////////////
364   -// Bug 716801 FAIL if any/only Location: header is blank
365   -test_flags[12] = CL_EXPECT_FAILURE;
  364 +// Bug 716801 OK for Location: header to be blank
366 365
367 366 function handler12(metadata, response)
368 367 {
369 368 var body = "012345678901234567890123456789";
370 369 response.seizePower();
371   - response.write("HTTP/1.0 301 Moved\r\n");
  370 + response.write("HTTP/1.0 200 OK\r\n");
372 371 response.write("Content-Type: text/plain\r\n");
373 372 response.write("Content-Length: 30\r\n");
374 373 response.write("Location:\r\n");
@@ -380,7 +379,8 @@ function handler12(metadata, response)
380 379
381 380 function completeTest12(request, data, ctx)
382 381 {
383   - do_check_eq(request.status, Components.results.NS_ERROR_CORRUPTED_CONTENT);
  382 + do_check_eq(request.status, Components.results.NS_OK);
  383 + do_check_eq(30, data.length);
384 384
385 385 run_test_number(13);
386 386 }
@@ -567,5 +567,33 @@ function completeTest19(request, data, ctx)
567 567 do_check_eq(request.status, Components.results.NS_OK);
568 568 do_check_eq(30, data.length);
569 569
  570 + run_test_number(20);
  571 +}
  572 +
  573 +////////////////////////////////////////////////////////////////////////////////
  574 +// FAIL if 1st Location: header is blank, followed by non-blank
  575 +test_flags[20] = CL_EXPECT_FAILURE;
  576 +
  577 +function handler20(metadata, response)
  578 +{
  579 + var body = "012345678901234567890123456789";
  580 + response.seizePower();
  581 + response.write("HTTP/1.0 301 Moved\r\n");
  582 + response.write("Content-Type: text/plain\r\n");
  583 + response.write("Content-Length: 30\r\n");
  584 + // redirect to previous test handler that completes OK: test 4
  585 + response.write("Location:\r\n");
  586 + response.write("Location: http://localhost:4444" + testPathBase + "4\r\n");
  587 + response.write("Connection: close\r\n");
  588 + response.write("\r\n");
  589 + response.write(body);
  590 + response.finish();
  591 +}
  592 +
  593 +function completeTest20(request, data, ctx)
  594 +{
  595 + do_check_eq(request.status, Components.results.NS_ERROR_CORRUPTED_CONTENT);
  596 +
570 597 endTests();
571 598 }
  599 +
90 netwerk/test/unit/test_redirect_loop.js
... ... @@ -0,0 +1,90 @@
  1 +do_load_httpd_js();
  2 +
  3 +/*
  4 + * This xpcshell test checks whether we detect infinite HTTP redirect loops.
  5 + * We check loops with "Location:" set to 1) full URI, 2) relative URI, and 3)
  6 + * empty Location header (which resolves to a relative link to the original
  7 + * URI when the original URI ends in a slash).
  8 + */
  9 +
  10 +var httpServer = null;
  11 +
  12 +var fullLoopPath = "/fullLoop";
  13 +var fullLoopURI = "http://localhost:4444" + fullLoopPath;
  14 +
  15 +var relativeLoopPath = "/relativeLoop";
  16 +var relativeLoopURI = "http://localhost:4444" + relativeLoopPath;
  17 +
  18 +// must use directory-style URI, so empty Location redirects back to self
  19 +var emptyLoopPath = "/empty/";
  20 +var emptyLoopURI = "http://localhost:4444" + emptyLoopPath;
  21 +
  22 +function make_channel(url, callback, ctx) {
  23 + var ios = Cc["@mozilla.org/network/io-service;1"].
  24 + getService(Ci.nsIIOService);
  25 + return ios.newChannel(url, "", null);
  26 +}
  27 +
  28 +function fullLoopHandler(metadata, response)
  29 +{
  30 + response.setStatusLine(metadata.httpVersion, 301, "Moved");
  31 + response.setHeader("Location", "http://localhost:4444/fullLoop", false);
  32 +}
  33 +
  34 +function relativeLoopHandler(metadata, response)
  35 +{
  36 + response.setStatusLine(metadata.httpVersion, 301, "Moved");
  37 + response.setHeader("Location", "relativeLoop", false);
  38 +}
  39 +
  40 +function emptyLoopHandler(metadata, response)
  41 +{
  42 + // Comrades! We must seize power from the petty-bourgeois running dogs of
  43 + // httpd.js in order to reply with a blank Location header!
  44 + response.seizePower();
  45 + response.write("HTTP/1.0 301 Moved\r\n");
  46 + response.write("Location: \r\n");
  47 + response.write("Content-Length: 4\r\n");
  48 + response.write("\r\n");
  49 + response.write("oops");
  50 + response.finish();
  51 +}
  52 +
  53 +function testFullLoop(request, buffer)
  54 +{
  55 + do_check_eq(request.status, Components.results.NS_ERROR_REDIRECT_LOOP);
  56 +
  57 + var chan = make_channel(relativeLoopURI);
  58 + chan.asyncOpen(new ChannelListener(testRelativeLoop, null, CL_EXPECT_FAILURE),
  59 + null);
  60 +}
  61 +
  62 +function testRelativeLoop(request, buffer)
  63 +{
  64 + do_check_eq(request.status, Components.results.NS_ERROR_REDIRECT_LOOP);
  65 +
  66 + var chan = make_channel(emptyLoopURI);
  67 + chan.asyncOpen(new ChannelListener(testEmptyLoop, null, CL_EXPECT_FAILURE),
  68 + null);
  69 +}
  70 +
  71 +function testEmptyLoop(request, buffer)
  72 +{
  73 + do_check_eq(request.status, Components.results.NS_ERROR_REDIRECT_LOOP);
  74 +
  75 + httpServer.stop(do_test_finished);
  76 +}
  77 +
  78 +function run_test()
  79 +{
  80 + httpServer = new nsHttpServer();
  81 + httpServer.registerPathHandler(fullLoopPath, fullLoopHandler);
  82 + httpServer.registerPathHandler(relativeLoopPath, relativeLoopHandler);
  83 + httpServer.registerPathHandler(emptyLoopPath, emptyLoopHandler);
  84 + httpServer.start(4444);
  85 +
  86 + var chan = make_channel(fullLoopURI);
  87 + chan.asyncOpen(new ChannelListener(testFullLoop, null, CL_EXPECT_FAILURE),
  88 + null);
  89 + do_test_pending();
  90 +}
1  netwerk/test/unit/xpcshell.ini
@@ -154,6 +154,7 @@ skip-if = os == "android"
154 154 [test_redirect_canceled.js]
155 155 [test_redirect_failure.js]
156 156 [test_redirect_passing.js]
  157 +[test_redirect_loop.js]
157 158 [test_reentrancy.js]
158 159 [test_reopen.js]
159 160 [test_resumable_channel.js]

0 comments on commit c7d5f4e

Please sign in to comment.
Something went wrong with that request. Please try again.