actix-web icon indicating copy to clipboard operation
actix-web copied to clipboard

get route macro does not result in HEAD request support

Open ijackson opened this issue 2 years ago • 6 comments

Example test case

#[get("/foo")]
async fn foo() -> impl Responder {
  "foo\r\n"
}

Current Behavior

$ curl -v http://localhost:8080/foo
...
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Expire in 200 ms for 4 (transfer 0x55e8ff381fb0)
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /foo HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.64.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< content-length: 5
< content-type: text/plain; charset=utf-8
< date: Sat, 19 Mar 2022 18:30:00 GMT
< 
foo
* Connection #0 to host localhost left intact
$ curl -v -I http://localhost:8080/foo
...
* Expire in 0 ms for 1 (transfer 0x55f159685fb0)
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Expire in 200 ms for 4 (transfer 0x55f159685fb0)
* Connected to localhost (127.0.0.1) port 8080 (#0)
> HEAD /foo HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.64.0
> Accept: */*
> 
< HTTP/1.1 404 Not Found
HTTP/1.1 404 Not Found
< content-length: 0
content-length: 0
< date: Sat, 19 Mar 2022 18:30:58 GMT
date: Sat, 19 Mar 2022 18:30:58 GMT

< 
* Connection #0 to host localhost left intact
$ 

Expected Behavior

The HEAD request is implemented according to the RFC specification.

I can obtain the expected behaviour by writing:

#[route("/foo", method="GET", method="HEAD")]

but this is not ergonomic.

Possible Solutions

  1. Make the get macro work like #[route("/foo", method="GET", method="HEAD")] (and adjust documentation accordingly). This would mean that if an application author writes the obvious code, it is also correct.

  2. Provide a new macro that behaves like #[route("/foo", method="GET", method="HEAD")], and try to discourage application authors from using #[get].

References

MDN, RFC7231 4.3.2.

Your Environment

  • Rust Version (I.e, output of rustc -V): rustc 1.61.0-nightly (1bfe40d11 2022-03-18)
  • Actix Web Version: 4.0.1

ijackson avatar Mar 19 '22 18:03 ijackson

There is more into this than just macro attributes.

actix-web does not filter out response body(http/2 specific) for HEAD method. So your handler have to treat HEAD differently than GET to make sure it always behave correctly for now.

Until a consistent behaviour for handling HEAD is formed inside actix-web a change on codegen could bring unexpected issue.

fakeshadow avatar Mar 20 '22 01:03 fakeshadow

actix-web does not filter out response body(http/2 specific) for HEAD method

Hrrm. It works correctly for http1; I just tried it again by hand. That sounds like a bug for the http2 support. I don't see an issue for it. Is there one?

But, having said that, it seems to work for me, when I test it with curl:

I tried

#[route("/foo", method="GET", method="HEAD")]
async fn foo() -> impl Responder { "foo\r\n" }

and then strace -e read=5 curl --http2 -I -v http://localhost:8080/foo. curl printed the expected information, and the strace shows that the content length is not included:

 | 00000  48 54 54 50 2f 31 2e 31  20 32 30 30 20 4f 4b 0d  HTTP/1.1 200 OK. |
 | 00010  0a 63 6f 6e 74 65 6e 74  2d 6c 65 6e 67 74 68 3a  .content-length: |
 | 00020  20 35 0d 0a 63 6f 6e 74  65 6e 74 2d 74 79 70 65   5..content-type |
 | 00030  3a 20 74 65 78 74 2f 70  6c 61 69 6e 3b 20 63 68  : text/plain; ch |
 | 00040  61 72 73 65 74 3d 75 74  66 2d 38 0d 0a 78 2d 63  arset=utf-8..x-c |
 | 00050  6f 6e 74 65 6e 74 2d 74  79 70 65 2d 6f 70 74 69  ontent-type-opti |
 | 00060  6f 6e 73 3a 20 6e 6f 73  6e 69 66 66 0d 0a 64 61  ons: nosniff..da |
 | 00070  74 65 3a 20 53 75 6e 2c  20 32 30 20 4d 61 72 20  te: Sun, 20 Mar  |
 | 00080  32 30 32 32 20 31 38 3a  35 34 3a 32 31 20 47 4d  2022 18:54:21 GM |
 | 00090  54 0d 0a 0d 0a                                    T....            |

As a check, I ran the command without the -I and then the foo appears in the response as expected.

ijackson avatar Mar 20 '22 18:03 ijackson

Oh wait. That doesn't seem to be doing http2 in fact.

ijackson avatar Mar 20 '22 18:03 ijackson

And

$ curl --http2 --http2-prior-knowledge -v http://localhost:8080/foo
* Expire in 0 ms for 6 (transfer 0x5593489ebfb0)
...
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Expire in 200 ms for 4 (transfer 0x5593489ebfb0)
* Connected to localhost (127.0.0.1) port 8080 (#0)
* Using HTTP2, server supports multi-use
* Connection state changed (HTTP/2 confirmed)
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* Using Stream ID: 1 (easy handle 0x5593489ebfb0)
> GET /foo HTTP/2
> Host: localhost:8080
> User-Agent: curl/7.64.0
> Accept: */*
> 
* http2 error: Remote peer returned unexpected data while we expected SETTINGS frame.  Perhaps, peer does not support HTTP/2 properly.
* Connection #0 to host localhost left intact
curl: (16) Error in the HTTP2 framing layer

which I don't understand. Maybe my test app (which started out as one of the actix examples) doesn't support http2 at all.

ijackson avatar Mar 20 '22 19:03 ijackson

I looked at this again, and I think what is happening is that actix does not support http2 outside TLS. For my application I think this means everything is fine, since I am running it behind a reverse proxy, which does the TLS.

Personally I think having HEAD requests work properly is a basic requirement for a web service, and HTTP/2 support is a nice-to-have. So I would like to definitively disable HTTP/2 support, and use the method workaround I give above.

Unfortunately there doesn't seem to be a way to disable HTTP/2 support?

ijackson avatar Mar 30 '22 22:03 ijackson

I’m running into this as well. Any progress on this issue?

bkolligs avatar Feb 19 '23 22:02 bkolligs