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

reverseproxy: avoid panic if failed to parse url #2848

Closed
wants to merge 1 commit into from

Conversation

aca
Copy link
Contributor

@aca aca commented Oct 31, 2019

1. What does this change do, exactly?

caddy reverse-proxy -from ':30000' -to '127.0.0.1:8384'

this commands panics when failed to parse error as it tries to write to nil pointer.

2. Please link to the relevant issues.

none

4. Checklist

  • I have written tests and verified that they fail without my change
  • I have squashed any insignificant commits
  • This change has comments explaining package types, values, functions, and non-obvious lines of code
  • I am willing to help maintain this change if there are issues with it later

@mholt
Copy link
Member

mholt commented Oct 31, 2019

Thanks for the PR!

I am going to make sure I can reproduce this first, because I still don't understand what is going on. I am not seeing the error. (See my comment in your issue.)

What is the exact version of Go you are using, please?

@aca
Copy link
Contributor Author

aca commented Oct 31, 2019

@mholt oh, this one is not related to issue #2849.
Could you try this command?

caddy reverse-proxy -from ':30000' -to '127.0.0.1:8384' 

I am quite convinced that this will panic regardless of go version.

@aca
Copy link
Contributor Author

aca commented Oct 31, 2019

@mholt

 » go version
go version go1.13.2 linux/amd64

@mholt
Copy link
Member

mholt commented Oct 31, 2019

Okay, sorry, I thought this was related to #2849.

I can reproduce this one. It's the one in #2849 that I cannot reproduce.

@mholt
Copy link
Member

mholt commented Oct 31, 2019

I ask for the Go version because Go made a breaking change (!!) to how URLs are parsed in Go 1.13.1.

@aca
Copy link
Contributor Author

aca commented Oct 31, 2019

@mholt oh god.. Didn't know that..

@mholt
Copy link
Member

mholt commented Oct 31, 2019

Okay cool, this one was easy to fix (I am pushing a separate commit, if that's alright, with a different fix based on the details I saw in the stack trace, etc).

Thanks for the report!

@mholt
Copy link
Member

mholt commented Oct 31, 2019

Pushed the fix in 8ef0a0b -- appreciate your help!

@mholt mholt added the v2 label Oct 31, 2019
@mholt mholt added this to the v2.0.0-beta9 milestone Oct 31, 2019
@aca aca deleted the hotfix branch November 24, 2019 18: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.

2 participants