Skip to content

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

2 participants