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

possible regression introduce by FaaSController #73

Closed
tg123 opened this issue Jun 27, 2021 · 3 comments
Closed

possible regression introduce by FaaSController #73

tg123 opened this issue Jun 27, 2021 · 3 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@tg123
Copy link
Contributor

tg123 commented Jun 27, 2021

lines below are removed in #59

ci = &cacheItem{ipFilterChan: path.ipFilterChain, path: path}
rules.putCacheItem(ctx, ci)
m.handleRequestWithCache(rules, ctx, ci)
return

d6af0dc#diff-9a718d32cef26863af1d2292e908365d40b9d2f761b2bc1bbde6ec743d3b8bdd

when you run the example in readme
https://github.com/megaease/easegress#create-an-httpserver-and-pipeline
you will always get 404 because of path.Headers === nil

the egress server will never touch any backend server if no header set in config object

@xxx7xxxx
Copy link
Contributor

Yes, we have fixed that problem, will merge it in the next PR. Thx

@benja-wu
Copy link
Contributor

benja-wu commented Jun 27, 2021

@tg123 Thanks

  • In the original PR, I notice the mux package has a potential problem for handling requests even in the header-not-match scenario. So I remove it and create a discussion related to it. But it had been merged without full discussion and testing.
  • For a stable usage scenario, please use the 1.0.0 tag version's Easegress, and I will open a new issue for this not feature-related modification next time.

BTW: the correct logic here should be if there are headers related setting, not match will cause a 404, else it is a path matching. We will fix it in the next PR.

@benja-wu benja-wu self-assigned this Jun 27, 2021
@benja-wu benja-wu added the bug Something isn't working label Jun 27, 2021
@benja-wu benja-wu added this to To do in Easegress Project via automation Jun 27, 2021
@benja-wu benja-wu added this to the v1.0.1 milestone Jun 27, 2021
@benja-wu
Copy link
Contributor

@tg123 We had launched the fixing PR #76, and in normal path-matching(Readme example) and header-matching(FaaSController) scenarios, Easegress can work as expected.

Thanks for reporting this bug again. :-)

Easegress Project automation moved this from To do to Done Jun 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

No branches or pull requests

3 participants