You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up from the discussion on #3524 (which fixed #3522, PUT errors being masked as 405).
RestWebServer::RoutingErrorHandler doesn't correctly implement REST semantics around 404/405/OPTIONS:
404 vs 405: A 404 Not Found should be returned only when the requested resource (route) doesn't exist on the server at all. A 405 Method Not Allowed should be returned when the resource exists but doesn't support the requested method. RoutingErrorHandler doesn't reliably distinguish these two cases across all methods — [rest] fix PUT errors masked as 405 by RoutingErrorHandler #3524 only fixed this for PUT, via a hardcoded mPutSupportedPaths allowlist populated through a RegisterPut() wrapper. The same routing-miss-vs-real-error ambiguity likely exists for other methods (e.g. DELETE/POST against a path that doesn't support them) since cpp-httplib's own routing-miss default status leaks through unmodified for those methods.
Allow header on 405: When returning 405, the Allow header must list the methods actually supported by the specific resource being requested, not a single static, server-wide list. The current implementation (aResponse.set_header("Allow", "GET, POST, PUT, DELETE, OPTIONS");) is wrong for the majority of resources, since most endpoints don't actually support all of those methods.
OPTIONS: The OPTIONS handling is only partially implemented across handlers, and only some of them set an Allow header in their response.
Suggested direction
Doing this properly likely requires each route's supported-method set to be introspectable from the router (e.g. extending the RegisterPut-style wrapper pattern to RegisterGet/RegisterPost/RegisterDelete/RegisterOptions, or building a small per-path method registry), so that:
a request to an unregistered path returns 404,
a request with an unsupported method on a registered path returns 405 with a correct, per-resource Allow header,
OPTIONS responses consistently report the same per-resource Allow header.
Description
Follow-up from the discussion on #3524 (which fixed #3522, PUT errors being masked as
405).RestWebServer::RoutingErrorHandlerdoesn't correctly implement REST semantics around404/405/OPTIONS:404 Not Foundshould be returned only when the requested resource (route) doesn't exist on the server at all. A405 Method Not Allowedshould be returned when the resource exists but doesn't support the requested method.RoutingErrorHandlerdoesn't reliably distinguish these two cases across all methods — [rest] fix PUT errors masked as 405 by RoutingErrorHandler #3524 only fixed this forPUT, via a hardcodedmPutSupportedPathsallowlist populated through aRegisterPut()wrapper. The same routing-miss-vs-real-error ambiguity likely exists for other methods (e.g.DELETE/POSTagainst a path that doesn't support them) since cpp-httplib's own routing-miss default status leaks through unmodified for those methods.405, theAllowheader must list the methods actually supported by the specific resource being requested, not a single static, server-wide list. The current implementation (aResponse.set_header("Allow", "GET, POST, PUT, DELETE, OPTIONS");) is wrong for the majority of resources, since most endpoints don't actually support all of those methods.OPTIONShandling is only partially implemented across handlers, and only some of them set anAllowheader in their response.Suggested direction
Doing this properly likely requires each route's supported-method set to be introspectable from the router (e.g. extending the
RegisterPut-style wrapper pattern toRegisterGet/RegisterPost/RegisterDelete/RegisterOptions, or building a small per-path method registry), so that:404,405with a correct, per-resourceAllowheader,OPTIONSresponses consistently report the same per-resourceAllowheader.References
cc @3oris @jwhui