Skip to content

Ignore trailing slash - #32

Merged
tigerwill90 merged 13 commits into
masterfrom
feat/ignore-trailling-slash
Jun 24, 2024
Merged

tigerwill90 merged 13 commits into
masterfrom
feat/ignore-trailling-slash

Conversation

@tigerwill90

@tigerwill90 tigerwill90 commented Jun 23, 2024 •

Copy link
Copy Markdown
Collaborator

Summary

This pull request introduces a new feature to Fox: the ability to ignore trailing slashes in URL paths. While redirecting or ignoring trailing slashes for an API endpoint is not typically advised, it may be desirable for websites. Both methods have their pros and cons, and most routers, including Fox, choose to redirect the client with a 301 status code for trailing slashes.

For example, https://apple.com/iphone redirects you to https://apple.com/iphone/ with a 301 Moved Permanently, while https://github.com/tigerwill90/fox/ and https://github.com/tigerwill90/fox both match the same resource without any redirection.

This new feature allows the router to handle requests regardless of the presence or absence of a trailing slash.

Details

  • In contrast to the existing redirect trailing slashes feature, which redirects the client if a route with or without a trailing slash matches, this new feature allows the router to handle the request directly.
  • This feature operates at no extra cost. There is no need to perform an additional lookup to find if a path would match with or without the trailing slash.
  • The WithIgnoreTrailingSlash and WithRedirectTrailingSlash options are mutually exclusive. If both are enabled, WithIgnoreTrailingSlash takes precedence.

Example

In this example, all the following requests match a handler:

  • /articles
  • /articles/
  • /articles/12
  • /articles/51/
f := fox.New(fox.WithIgnoreTrailingSlash(true))

f.MustHandle(http.MethodGet, "/articles", func(c fox.Context) {
	_ = c.String(http.StatusOK, "Here is a list of articles\n")
})
f.MustHandle(http.MethodGet, "/articles/{id}", func(c fox.Context) {
	_ = c.String(http.StatusOK, "Here is article %s\n", c.Param("id"))
})

Consistent Behavior with Tree API

The Tree API allows the user to access lower-level functions like Match, Has, Methods, and Lookup, which may be useful for managing routes at runtime. These methods will also ignore the trailing slash if the option is enabled.

Bug Fix

An edge case has been fixed for both the redirect trailing slash and the new ignore trailing slash feature. In some scenarios, the router would miss a possible redirect recommendation if the path exactly matched an intermediary node (without a handler).

/foo [leaf=/foo]
     /
         b/ [leaf=/foo/b/]
         x/ [leaf=/foo/x/]

In this circumstance, /foo/ can be redirected to /foo but was previously missed by the router. Now, the lookup function correctly handles this edge case.

Benchmark Differential

Expect none or negligible impact on performance.

goos: linux
goarch: amd64
pkg: github.com/tigerwill90/fox
cpu: Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz
                    │    old.txt    │               new.txt               │
                    │    sec/op     │    sec/op     vs base               │
StaticAll-16           11.68µ ±  1%   11.85µ ±  2%       ~ (p=0.089 n=10)
GithubParamsAll-16     82.21n ±  2%   83.72n ±  1%  +1.84% (p=0.000 n=10)
OverlappingRoute-16    99.98n ±  1%   96.13n ±  1%  -3.85% (p=0.000 n=10)
StaticParallel-16     10.105n ±  1%   9.345n ±  2%  -7.53% (p=0.001 n=10)
CatchAll-16            31.71n ±  1%   31.87n ±  2%       ~ (p=0.128 n=10)
CatchAllParallel-16    5.742n ± 13%   5.712n ± 24%       ~ (p=0.739 n=10)
CloneWith-16           65.60n ±  1%   67.57n ±  2%  +3.00% (p=0.000 n=10)
geomean                73.50n         72.92n        -0.79%

Path to v1.0.0

We are gradually moving towards the v1.0.0 release. While some API inconsistencies remain, along with necessary code cleanup and documentation updates, we anticipate reaching v1.0.0 during the summer.

@tigerwill90
tigerwill90 marked this pull request as ready for review June 23, 2024 11:49
@pawndev pawndev added the enhancement New feature or request label Jun 23, 2024
pawndev
pawndev previously approved these changes Jun 23, 2024

@pawndev pawndev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice feature ! Thanks for that, buddy 🙏

@codecov

codecov Bot commented Jun 23, 2024 •

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.23%. Comparing base (2ba5953) to head (004626f).

❗ Current head 004626f differs from pull request most recent head 7c00186

Please upload reports for the commit 7c00186 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #32      +/-   ##
==========================================
+ Coverage   88.08%   88.23%   +0.15%     
==========================================
  Files          12       12              
  Lines        1678     1726      +48     
==========================================
+ Hits         1478     1523      +45     
- Misses        169      171       +2     
- Partials       31       32       +1     
Flag Coverage Δ
coverage.txt 88.23% <100.00%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pawndev
pawndev previously approved these changes Jun 24, 2024
Comment thread fox.go
// Shouldn't send the body for POST or HEAD; that leaves GET.
if !hadCT && r.Method == "GET" {
body := "<a href=\"" + htmlEscape(path) + "\">" + http.StatusText(code) + "</a>.\n"
_, _ = fmt.Fprintln(w, body)

Check warning

Code scanning / CodeQL

Reflected cross-site scripting

Cross-site scripting vulnerability due to [user-provided value](1). Cross-site scripting vulnerability due to [user-provided value](2). Cross-site scripting vulnerability due to [user-provided value](3).
@tigerwill90
tigerwill90 merged commit 670e306 into master Jun 24, 2024
@tigerwill90
tigerwill90 deleted the feat/ignore-trailling-slash branch June 24, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants