short url: ensure absolute path isn't persisted by jbudz · Pull Request #6581 · elastic/kibana

jbudz · 2016-03-18T21:10:31Z

The /shorten relative url was calculated by combing protocol, host, port, base url and removing the match from an absolute url. In cases when port was set to 443 or 80, the absolute url may not contain port causing the match to fail and resulting in absolute urls being persisted.

Instead of matching and removing, this parses the absolute url passed and constructs a relative url from the parts.

I changed shortenUrl to return an angular promise - I wasn't able to get past await when using $httpBackend to mock test responses.

Fixes #6127, although this is a different bug than the original issue.

spalger · 2016-03-18T22:34:36Z

src/ui/public/share/__tests__/url_shortener.js

would like to see a test with urls that have basepaths, query strings, no hash, etc.

spalger · 2016-03-18T22:34:58Z

Can we add some validation server side?

spalger · 2016-03-18T22:36:16Z

src/ui/public/share/lib/url_shortener.js

I suggest using url.format()

jbudz · 2016-03-21T22:57:24Z

jenkins, test it

jbudz · 2016-03-21T23:20:13Z

jenkins, test it

spalger · 2016-03-22T03:13:25Z

LGTM

epixa · 2016-03-22T18:37:55Z

LGTM

short url: ensure absolute path isn't persisted

[Commit 1] short url: ensure absolute path isn't persisted Original sha: a8c1305 Authored by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-18T20:02:53Z Committed by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T15:57:16Z [Commit 2] [short url] use url.format when creating /goto link Original sha: 2a71673 Authored by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T14:09:04Z Committed by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T15:57:16Z [Commit 3] [short url] Add tests for query strings, no hashes Original sha: 690a140 Authored by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T14:42:10Z Committed by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T15:57:16Z [Commit 4] [short url] Cleanup Original sha: 49d297f Authored by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T15:09:35Z Committed by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T15:57:16Z [Commit 5] [short url] Add server tests Original sha: a612a0e Authored by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T15:30:12Z Committed by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T18:12:25Z

…/6127 [backport] PR #6581 to 4.x

[Commit 1] short url: ensure absolute path isn't persisted Original sha: a8c1305 Authored by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-18T20:02:53Z Committed by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T15:57:16Z [Commit 2] [short url] use url.format when creating /goto link Original sha: 2a71673 Authored by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T14:09:04Z Committed by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T15:57:16Z [Commit 3] [short url] Add tests for query strings, no hashes Original sha: 690a140 Authored by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T14:42:10Z Committed by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T15:57:16Z [Commit 4] [short url] Cleanup Original sha: 49d297f Authored by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T15:09:35Z Committed by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T15:57:16Z [Commit 5] [short url] Add server tests Original sha: a612a0e Authored by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T15:30:12Z Committed by Jonathan Budzenski <jbudz@users.noreply.github.com> on 2016-03-21T18:12:25Z

…/6127 [backport] PR #6581 to 4.5

…es/6127 [backport] PR #6581 to 4.5.0

…/6127 [backport] PR #6581 to 4.4

jbudz added review v4.5.0 v5.0.0 v4.4.3 labels Mar 18, 2016

jbudz assigned BigFunger Mar 18, 2016

spalger reviewed Mar 18, 2016
View reviewed changes

src/ui/public/share/lib/url_shortener.js Outdated

Copy link
Copy Markdown

Contributor

spalger Mar 18, 2016

Choose a reason for hiding this comment

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

I suggest using url.format()

jbudz force-pushed the issues/6127 branch from d9f26d2 to c2f7a9e Compare March 21, 2016 15:42

jbudz added 5 commits March 21, 2016 10:57

short url: ensure absolute path isn't persisted

a8c1305

[short url] use url.format when creating /goto link

2a71673

[short url] Add tests for query strings, no hashes

690a140

[short url] Cleanup

49d297f

[short url] Add server tests

a612a0e

jbudz force-pushed the issues/6127 branch from c2f7a9e to a612a0e Compare March 21, 2016 18:13

epixa assigned epixa and unassigned BigFunger Mar 22, 2016

epixa added a commit that referenced this pull request Mar 22, 2016

Merge pull request #6581 from jbudz/issues/6127

d66df55

short url: ensure absolute path isn't persisted

epixa merged commit d66df55 into elastic:master Mar 22, 2016

epixa added v4.5.1 v4.6.0 labels Mar 22, 2016

This was referenced Mar 22, 2016

[backport] PR #6581 to 4.x #6596

Merged

[backport] PR #6581 to 4.5.0 #6597

Merged

[backport] PR #6581 to 4.4 #6598

Merged

[backport] PR #6581 to 4.5 #6599

Merged

epixa added a commit that referenced this pull request Mar 22, 2016

Merge pull request #6596 from elastic/jasper/backport/6581-4.x-issues…

3ed5902

…/6127 [backport] PR #6581 to 4.x

epixa added a commit that referenced this pull request Mar 22, 2016

Merge pull request #6599 from elastic/jasper/backport/6581-4.5-issues…

4e261e2

…/6127 [backport] PR #6581 to 4.5

epixa added a commit that referenced this pull request Mar 22, 2016

Merge pull request #6597 from elastic/jasper/backport/6581-4.5.0-issu…

13229cf

…es/6127 [backport] PR #6581 to 4.5.0

epixa added a commit that referenced this pull request Mar 22, 2016

Merge pull request #6598 from elastic/jasper/backport/6581-4.4-issues…

5d4bec9

…/6127 [backport] PR #6581 to 4.4

spalger mentioned this pull request Mar 31, 2016

Short URL expands to wrong URL #6609

Closed

epixa added v5.0.0-alpha1 and removed v5.0.0-alpha1 labels Mar 31, 2016

epixa added v4.5.2 and removed v4.5.1 labels May 14, 2016

epixa removed the v4.4.3 label Aug 4, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

short url: ensure absolute path isn't persisted#6581

short url: ensure absolute path isn't persisted#6581
epixa merged 5 commits intoelastic:masterfrom
jbudz:issues/6127

jbudz commented Mar 18, 2016

Uh oh!

spalger Mar 18, 2016

Uh oh!

spalger Mar 21, 2016

Uh oh!

spalger commented Mar 18, 2016

Uh oh!

spalger Mar 18, 2016

Uh oh!

jbudz commented Mar 21, 2016

Uh oh!

jbudz commented Mar 21, 2016

Uh oh!

spalger commented Mar 22, 2016

Uh oh!

epixa commented Mar 22, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jbudz commented Mar 18, 2016

Uh oh!

spalger Mar 18, 2016

Choose a reason for hiding this comment

Uh oh!

spalger Mar 21, 2016

Choose a reason for hiding this comment

Uh oh!

spalger commented Mar 18, 2016

Uh oh!

spalger Mar 18, 2016

Choose a reason for hiding this comment

Uh oh!

jbudz commented Mar 21, 2016

Uh oh!

jbudz commented Mar 21, 2016

Uh oh!

spalger commented Mar 22, 2016

Uh oh!

epixa commented Mar 22, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants