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.

Sign up for GitHub

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

Jump to bottom

cmd/alertmanager: reject invalid external URLs #1960

Merged
merged 3 commits into from
Jul 22, 2019
Merged

cmd/alertmanager: reject invalid external URLs #1960

merged 3 commits into from
Jul 22, 2019

Conversation

simonpasquier
Copy link
Member

Closes #1912

Using --web.external-url=foo:123 instead of --web.external-url=http://foo:123 was accepted before but it's (almost) never what you want as html/template will escape the value to #ZgotmplZ because it only accepts http://..., https://... or mailto://... links.

brian-brazil
brian-brazil reviewed Jul 11, 2019
View reviewed changes
Copy link
Contributor

@brian-brazil brian-brazil left a comment

Choose a reason for hiding this comment

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

I think you may have forgotten to push some changes.

cmd/alertmanager/main.go Outdated Show resolved Hide resolved
api/api.go Outdated Show resolved Hide resolved
cmd/alertmanager/main.go Outdated Show resolved Hide resolved
@brian-brazil
Copy link
Contributor

👍

mxinden
mxinden reviewed Jul 17, 2019
View reviewed changes
cmd/alertmanager/main.go Outdated Show resolved Hide resolved
@simonpasquier
cmd/alertmanager: reject invalid external URLs
4cd83ad 
Signed-off-by: Simon Pasquier <spasquie@redhat.com>
@simonpasquier
Address Brian's comments
0dd11af 
Signed-off-by: Simon Pasquier <spasquie@redhat.com>
@simonpasquier
Simplify the code according to Max's feedback
0baa06c 
Signed-off-by: Simon Pasquier <spasquie@redhat.com>
@simonpasquier simonpasquier force-pushed the reject-invalid-external-url branch from e0610ab to 0baa06c Compare July 18, 2019 10:04
mxinden
mxinden approved these changes Jul 22, 2019
View reviewed changes
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@simonpasquier simonpasquier merged commit 78c9ebc into prometheus:master Jul 22, 2019
@simonpasquier simonpasquier deleted the reject-invalid-external-url branch July 22, 2019 12:35
DuskEagle pushed a commit to DuskEagle/alertmanager that referenced this pull request Aug 1, 2019
@simonpasquier @DuskEagle
cmd/alertmanager: reject invalid external URLs (prometheus#1960)
fcd8331 
* cmd/alertmanager: reject invalid external URLs

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

* Address Brian's comments

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

* Simplify the code according to Max's feedback

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
@monotek monotek mentioned this pull request Jan 8, 2020
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brian-brazil brian-brazil brian-brazil left review comments

@mxinden mxinden mxinden approved these changes

@stuartnelson3 stuartnelson3 Awaiting requested review from stuartnelson3

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Template email not replacing external and generator URL
3 participants
@simonpasquier @brian-brazil @mxinden