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

fix(API): encodeURIComponent for each path element #8646

Merged
merged 2 commits into from
Jan 20, 2021
Merged

fix(API): encodeURIComponent for each path element #8646

merged 2 commits into from
Jan 20, 2021

Conversation

christopherthielen
Copy link
Contributor

@christopherthielen christopherthielen commented Oct 12, 2020 •
edited
Loading

Re-submittal of reverted PR #8586

I chose to make the first .one()/.all() exempt from encoding so legacy code like API.one('/foo/bar').one(param) will still work.

@christopherthielen christopherthielen mentioned this pull request Oct 16, 2020
Closed
@christopherthielen christopherthielen mentioned this pull request Nov 30, 2020
Merged
@christopherthielen christopherthielen added the ready to merge Reviewed and ready for merge label Jan 20, 2021
@mergify mergify bot merged commit e78115e into spinnaker:master Jan 20, 2021
caseyhebebrand added a commit that referenced this pull request Jan 21, 2021
@caseyhebebrand
chore(core): publish core@0.0.542
5b0ca27 
b0139a6 feat(core/loadBalancer): Filter by load balancer type (#8850)
5e45b85 feat(core/pipeline): Add the ability to disable base os options (#8851)
dde8513 fix(core/pipelines): break and wrap long comments text (#8748)
e78115e fix(API): encodeURIComponent for each path() element (#8646)
@caseyhebebrand caseyhebebrand mentioned this pull request Jan 21, 2021
Merged
mergify bot pushed a commit that referenced this pull request Jan 21, 2021
@caseyhebebrand @mergify
chore(core): publish core@0.0.542
35e0131 
b0139a6 feat(core/loadBalancer): Filter by load balancer type (#8850)
5e45b85 feat(core/pipeline): Add the ability to disable base os options (#8851)
dde8513 fix(core/pipelines): break and wrap long comments text (#8748)
e78115e fix(API): encodeURIComponent for each path() element (#8646)
@jervi
Copy link
Contributor

jervi commented Jan 26, 2021

@christopherthielen This PR breaks fetching of builds from (some?) CI systems, at least Travis but probably also Jenkins. To reproduce, start a manual execution of a pipeline with a build trigger set.
image
This is the call that fails: https://github.com/spinnaker/deck/blob/master/app/scripts/modules/core/src/ci/igor.service.ts#L36
job here can contain slashes that should not be urlEncoded.

@christopherthielen christopherthielen deleted the url-encoded branch February 5, 2021 01:05
@christopherthielen christopherthielen mentioned this pull request Feb 8, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caseyhebebrand caseyhebebrand caseyhebebrand approved these changes

@alanmquach alanmquach Awaiting requested review from alanmquach

@vigneshm vigneshm Awaiting requested review from vigneshm

Assignees
No one assigned
Labels
ready to merge Reviewed and ready for merge target-release/1.25
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@christopherthielen @jervi @caseyhebebrand @spinnakerbot