Skip to content

Commit

Permalink
fix #127 only trust the computed Content-Length header
Browse files Browse the repository at this point in the history
  • Loading branch information
shenfeng committed Mar 18, 2014
1 parent 59eddc7 commit 9b67cd8
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 16 deletions.
17 changes: 14 additions & 3 deletions src/java/org/httpkit/HeaderMap.java
Expand Up @@ -26,7 +26,7 @@ public boolean isEmpty() {
private Object arrays[] = new Object[INIT_SIZE * 2];

public void put(String key, Object obj) {
final int total = size << 1;
final int total = size * 2;
if (total == arrays.length) {
arrays = Arrays.copyOf(arrays, arrays.length * 2);
}
Expand All @@ -35,6 +35,17 @@ public void put(String key, Object obj) {
size += 1;
}

public void putOrReplace(String key, Object obj) {
final int total = size * 2; // * 2
for (int i = 0; i < total; i += 2) {
if (key.equals(arrays[i])) {
arrays[i + 1] = obj;// replace
return;
}
}
put(key, obj);
}

public Object get(String key) {
final int total = size << 1; // * 2
for (int i = 0; i < total; i += 2) {
Expand All @@ -46,7 +57,7 @@ public Object get(String key) {
}

public boolean containsKey(String key) {
final int total = size << 1; // * 2
final int total = size * 2; // * 2
for (int i = 0; i < total; i += 2) {
if (key.equals(arrays[i])) {
return true;
Expand All @@ -70,7 +81,7 @@ public static HeaderMap camelCase(Map<String, Object> map) {
}

public void encodeHeaders(DynamicBytes bytes) {
final int total = size << 1;
final int total = size * 2;
for (int i = 0; i < total; i += 2) {
String k = (String) arrays[i];
Object v = arrays[i + 1];
Expand Down
5 changes: 3 additions & 2 deletions src/java/org/httpkit/HttpUtils.java
Expand Up @@ -430,9 +430,10 @@ public static ByteBuffer[] HttpEncode(int status, HeaderMap headers, Object body
// only write length if not chunked
if (!CHUNKED.equals(headers.get("Transfer-Encoding"))) {
if (bodyBuffer != null) {
headers.put(CL, Integer.toString(bodyBuffer.remaining()));
// trust the computed length
headers.putOrReplace(CL, Integer.toString(bodyBuffer.remaining()));
} else {
headers.put(CL, "0");
headers.putOrReplace(CL, "0");
}
}
} catch (IOException e) {
Expand Down
14 changes: 9 additions & 5 deletions src/java/org/httpkit/client/HttpClient.java
Expand Up @@ -242,10 +242,14 @@ public void exec(String url, RequestConfig cfg, SSLEngine engine, IRespListener

// copy to modify, normalize header
HeaderMap headers = HeaderMap.camelCase(cfg.headers);
headers.put("Host", HttpUtils.getHost(uri));

if (!headers.containsKey("Accept")) // allow override
headers.put("Accept", "*/*");
if (!headers.containsKey("Host")) // if caller set it explicitly, let he do it
headers.put("Host", HttpUtils.getHost(uri));
/**
* commented on 2014/3/18: Accept is not required
*/
// if (!headers.containsKey("Accept")) // allow override
// headers.put("Accept", "*/*");
if (!headers.containsKey("User-Agent")) // allow override
headers.put("User-Agent", RequestConfig.DEFAULT_USER_AGENT); // default
if (!headers.containsKey("Accept-Encoding"))
Expand Down Expand Up @@ -277,9 +281,9 @@ private ByteBuffer[] encode(HttpMethod method, HeaderMap headers, Object body,
ByteBuffer bodyBuffer = HttpUtils.bodyBuffer(body);

if (body != null) {
headers.put("Content-Length", Integer.toString(bodyBuffer.remaining()));
headers.putOrReplace("Content-Length", Integer.toString(bodyBuffer.remaining()));
} else {
headers.put("Content-Length", "0");
headers.putOrReplace("Content-Length", "0");
}
DynamicBytes bytes = new DynamicBytes(196);
bytes.append(method.toString()).append(SP).append(HttpUtils.getPath(uri));
Expand Down
23 changes: 17 additions & 6 deletions test/org/httpkit/server_test.clj
Expand Up @@ -106,11 +106,11 @@
(GET "/headers" [] many-headers-handler)
(ANY "/spec" [] (fn [req] (pr-str (dissoc req :body :async-channel))))
(GET "/string" [] (fn [req] {:status 200
:headers {"Content-Type" "text/plain"}
:body "Hello World"}))
:headers {"Content-Type" "text/plain"}
:body "Hello World"}))
(GET "/iseq" [] (fn [req] {:status 200
:headers {"Content-Type" "text/plain"}
:body (range 1 10)}))
:headers {"Content-Type" "text/plain"}
:body (range 1 10)}))
(GET "/file" [] (wrap-file-info file-handler))
(GET "/ws" [] (fn [req]
(with-channel req con
Expand All @@ -119,10 +119,13 @@
(GET "/inputstream" [] inputstream-handler)
(POST "/multipart" [] multipart-handler)
(POST "/chunked-input" [] (fn [req] {:status 200
:body (str (:content-length req))}))
:body (str (:content-length req))}))
(GET "/length" [] (fn [req]
(let [l (-> req :params :length to-int)]
(subs const-string 0 l))))
{:status 200
;; this is wrong, but server should correct it
:headers {"content-length" 10000}
:body (subs const-string 0 l)})))
(GET "/null" [] (fn [req] {:status 200 :body nil}))
(GET "/demo" [] streaming-demo)

Expand Down Expand Up @@ -200,6 +203,14 @@
(is (= (:status resp) 200))
(is (= length (count (:body resp)))))))

;; https://github.com/http-kit/http-kit/issues/127
(deftest test-wrong-content-length
(doseq [length (range 1 1000 333)] ; max 5m, many files
(let [uri (str "http://localhost:4347/length?length=" length)
resp (http/get uri)]
(is (= (:status resp) 200))
(is (= length (count (:body resp)))))))

(deftest test-body-iseq
(let [resp (http/get "http://localhost:4347/iseq")]
(is (= (:status resp) 200))
Expand Down

0 comments on commit 9b67cd8

Please sign in to comment.