Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solution to Issue #751 #773

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

roy-kim-33
Copy link

@roy-kim-33 roy-kim-33 commented Oct 11, 2024

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Go Version Update
  • Dependency Update

Description

Address Issue #751 by using method mismatch flag in Route.Match()

The flag allows Route.Match() to return MethodNotAllowed (405) from an earlier mismatch instead of returning NotFound (404) from a later mismatch (refer to example from #751).

For nested endpoints,

r := mux.NewRouter()
subrouter := r.PathPrefix("/prefix").Subrouter()
subrouter.HandleFunc("/foo", fooHandler).Methods("POST")
subrouter.HandleFunc("/foo", fooIdHandler).Methods("GET").Queries("id", "{id:[0-9]+}")

The following results show some edge cases:
PUT /v1/foo?id=1 returns 405 (match second endpoint, desired behavior)
GET /v1/foo?id=abc returns 404 (match second endpoint, 400 on second endpoint preferred)
POST /v1/foo?id=abc returns 200 (match first endpoint, 405 on second endpoint might be preferred)
POST /v1/foo?id=1 returns 200 (match first endpoint, 405 on second endpoint might be preferred)
PUT /v1/foo returns 404 (match second endpoint, 405 on first endpoint preferred)
GET /v1/foo returns 404 (match second endpoint, 400 on first or 405 on second preferred)

These edge cases can be partially resolved(?) by declaring endpoints in descending order of specificity.

subrouter.HandleFunc("/foo", fooIdHandler).Methods("GET").Queries("id", "{id:[0-9]+}")
subrouter.HandleFunc("/foo", fooHandler).Methods("POST")

PUT /v1/foo?id=1 returns 405 (match first endpoint, desired behavior)
GET /v1/foo?id=abc returns 405 (match second endpoint, 400 on first endpoint preferred)
POST /v1/foo?id=abc returns 200 (match second endpoint, 405 on first endpoint might be preferred)
POST /v1/foo?id=1 returns 200 (match second endpoint, 405 on first endpoint might be preferred)
PUT /v1/foo returns 405 (match second endpoint, desired behavior)
GET /v1/foo returns 405 (match second endpoint, 400 on first endpoint might be preferred)

NOTE: might be is a soft suggestion

Related Tickets & Documents

  • Related Issue #
  • Closes #

Added/updated tests?

  • Yes
  • No, and this is why: existing test already tests this and passes, but, for an unknown reason, when using the package (by importing, not within the package), the test fails.
  • I need help with writing tests

Run verifications and test

  • make verify is passing
  • make test is passing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant