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

Fixed panic when Router#Find fails on Param paths #1659

Merged

Conversation

pafuent
Copy link
Contributor

@pafuent pafuent commented Oct 25, 2020

Fixed panic when Router#Find fails to find a route that could match a Param route that only have children routes and no root route.
e.g
/create
/:id/edit
/:id/active

Finding /creates results in panic because the router tree node that belongs to the param route :id don't have pnames on it. The childrens of :id (:id/edit and :id/active) have the pnames properly set, but those are not processed because /creates don't match on those paths.
This fix #1653

Fixed panic when Router#Find fails to find a route that could match a
Param route that only have children routes and no root route.
e.g
/create
/:id/edit
/:id/active

Finding /creates results in panic because the router tree node that
belongs to the param route :id don't have pnames on it. The childrens of
:id (:id/edit and :id/active) have the pnames properly set, but those
are not processed because /creates don't match on those paths.
@codecov
Copy link

codecov bot commented Oct 25, 2020

Codecov Report

Merging #1659 (c171855) into master (6caec30) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1659   +/-   ##
=======================================
  Coverage   84.98%   84.99%           
=======================================
  Files          29       29           
  Lines        1958     1959    +1     
=======================================
+ Hits         1664     1665    +1     
  Misses        187      187           
  Partials      107      107           
Impacted Files Coverage Δ
router.go 96.88% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6caec30...c171855. Read the comment docs.

@lammel
Copy link
Contributor

lammel commented Nov 20, 2020

Oh, forgot to review. This relates to #1661 and should be reviewed together.

@pafuent pafuent self-assigned this Nov 26, 2020
@lammel
Copy link
Contributor

lammel commented Dec 7, 2020

Please remove the go.sum changes in this PR as they are unrelated to the patch.
Will merge then.

@lammel lammel merged commit 8c27828 into labstack:master Dec 10, 2020
@pafuent pafuent deleted the panic_router_find_fails_on_params_with_no_root branch December 11, 2020 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTTP panic when using static and params routers
2 participants