wight-airmash / ab-server

2d multiplayer action game server for Node.js based on WebSocket communication.
https://airmash.online
MIT License
54 stars 21 forks source link

404 error on /ping #62

Closed BlackCrawler closed 4 years ago

BlackCrawler commented 4 years ago

I noticed many 404 errors in nginx logs.

140.x - - [12/Dec/2019:21:06:46 +0000] "HEAD /ping HTTP/1.1" 404 0 "https://airmash.online/" "Mozilla/5.0 (X11; CrOS x86_64 12499.66.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.106 Safari/537.36"
140.x - - [12/Dec/2019:21:06:46 +0000] "HEAD /ping HTTP/1.1" 404 0 "https://airmash.online/" "Mozilla/5.0 (X11; CrOS x86_64 12499.66.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.106 Safari/537.36"
140.x - - [12/Dec/2019:21:06:46 +0000] "HEAD /ping HTTP/1.1" 404 0 "https://airmash.online/" "Mozilla/5.0 (X11; CrOS x86_64 12499.66.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.106 Safari/537.36"
140.x - - [12/Dec/2019:21:06:46 +0000] "HEAD /ping HTTP/1.1" 404 0 "https://airmash.online/" "Mozilla/5.0 (X11; CrOS x86_64 12499.66.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.106 Safari/537.36"

I guess HEAD is not supported by the server, since curl -I http://localhost:3501/ping :

HTTP/1.1 404 Not Found
uWebSockets: v0.16
Content-Length: 0

And curl http://localhost:3501/ping

{"pong":1}
wight-airmash commented 4 years ago

Only GET method is implemented for the route "/ping".

RFC 7231 states that:

The server SHOULD send the same header fields in response to a HEAD request as it would have sent if the request had been a GET...

Where SHOULD is:

SHOULD This word, or the adjective "RECOMMENDED", mean that there may exist valid reasons in particular circumstances to ignore a particular item, but the full implications must be understood and carefully weighed before choosing a different course.

That is, RFC allows such implementations. I consider "/ping" as part of the API, the use of which is strictly limited to the needs of the application, specifically the GET method.

If someone adds the processing of missing methods to fully comply with RFC recommendations, such changes will of course be accepted.

congratulatio commented 4 years ago

I figured it was easier to change this on the frontend, no server changes required: https://github.com/airmash-refugees/airmash-frontend/commit/d817fa88afbb893feca8479471f09226881d3e57

(This change is okay because it doesn't matter to the frontend what response it gets or which HTTP verb is used to elicit that response, it just looks at the timing.)