fix(appname): encodeURIComponent for app name #8586
Conversation
There was a problem hiding this comment.
LGTM, but since it's core, I'd like to make sure others are aware of the changes, namely @christopherthielen and @maggieneterval
I hope there isn't a function that would try to parse the name from the url, that would be pretty funky if it did. I think it's unlikely since we're pretty good at passing around the app/pipeline contexts around in memory.
|
This seems like something we should do in the API code itself, not at every call site. I haven't investigated to find out if that is possible, however. |
|
That was my first option, but then I saw some code like this one calling the API methods with multiple uri components in one string instead of splitting them. |
|
@christopherthielen what do you think about fixing calls to API to not use more than one uri component and encode in API, vs the current implementation in the PR? |
|
I think we could do something like |
|
What if an application name includes |
|
I'm fairly certain that an application nameed That said, I think your solution is good and we should do that instead. |
|
We can also write a linter rule in https://github.com/spinnaker/deck/tree/master/packages/eslint-plugin that warns and auto-fixes this if somebody passes a string literal with a |
german-muzquiz
force-pushed
the
fix/appname-url-encoding
branch
from
b054b76
to
c4160ea
Compare
|
@christopherthielen I made the changes for encoding in API service and fixed calls to not use |
app/scripts/modules/core/src/pipeline/config/services/PipelineConfigService.ts
Outdated
Show resolved
Hide resolved
| const uri = '/plugins/deck/plugin-manifest.json'; | ||
| const loadPromise = API.one(uri) | ||
| const loadPromise = API.one(...uri.split('/').slice(1)) |
There was a problem hiding this comment.
API.one('plugins', 'deck', 'plugin-manifest.json');
?
Yes, of course. LGTM, just a couple minor suggestions |
…ConfigService.ts Co-authored-by: Chris Thielen <christopherthielen@users.noreply.github.com>
german-muzquiz
force-pushed
the
fix/appname-url-encoding
branch
from
32ed62f
to
513d0d4
Compare
80858e7 feat(core/managed): limit artifact versions to 30 (#8623) 90a2819 feat(core/managed): put constraints in a 'skipped' state on skipped versions (#8620) 815742d feat(core/managed): scroll to selected version in sidebar, update scroll containers (#8618) de24ef3 feat(core/managed): apply consistent sorting to resources (#8622) 3735483 fix(core/pipeline): Always enable "show revision history" in edit pipeline dialog (#8621) abffd84 fix(core/managed): fixup some details from new layout (#8619) f1bb04e fix(appname): encodeURIComponent for app name (#8586) 60c0c7b feat(validation): Allow app name validators to be overridden (#8584)
…e@0.0.516 docker@0.0.60 google@0.0.21 oracle@0.0.9 tencentcloud@0.0.6 (#8624) * chore(amazon): publish amazon@0.0.269 f1bb04e fix(appname): encodeURIComponent for app name (#8586) * chore(azure): publish azure@0.0.255 f1bb04e fix(appname): encodeURIComponent for app name (#8586) * chore(cloudfoundry): publish cloudfoundry@0.0.101 f1bb04e fix(appname): encodeURIComponent for app name (#8586) * chore(core): publish core@0.0.516 80858e7 feat(core/managed): limit artifact versions to 30 (#8623) 90a2819 feat(core/managed): put constraints in a 'skipped' state on skipped versions (#8620) 815742d feat(core/managed): scroll to selected version in sidebar, update scroll containers (#8618) de24ef3 feat(core/managed): apply consistent sorting to resources (#8622) 3735483 fix(core/pipeline): Always enable "show revision history" in edit pipeline dialog (#8621) abffd84 fix(core/managed): fixup some details from new layout (#8619) f1bb04e fix(appname): encodeURIComponent for app name (#8586) 60c0c7b feat(validation): Allow app name validators to be overridden (#8584) * chore(docker): publish docker@0.0.60 f1bb04e fix(appname): encodeURIComponent for app name (#8586) * chore(google): publish google@0.0.21 f1bb04e fix(appname): encodeURIComponent for app name (#8586) * chore(oracle): publish oracle@0.0.9 f1bb04e fix(appname): encodeURIComponent for app name (#8586) * chore(tencentcloud): publish tencentcloud@0.0.6 f1bb04e fix(appname): encodeURIComponent for app name (#8586)
This reverts commit f1bb04e.
…e@0.0.517 docker@0.0.61 google@0.0.22 oracle@0.0.10 tencentcloud@0.0.7 (#8630) * chore(amazon): publish amazon@0.0.270 885cd16 Revert "fix(appname): encodeURIComponent for app name (#8586)" (#8627) * chore(azure): publish azure@0.0.256 885cd16 Revert "fix(appname): encodeURIComponent for app name (#8586)" (#8627) * chore(cloudfoundry): publish cloudfoundry@0.0.102 885cd16 Revert "fix(appname): encodeURIComponent for app name (#8586)" (#8627) * chore(core): publish core@0.0.517 885cd16 Revert "fix(appname): encodeURIComponent for app name (#8586)" (#8627) * chore(docker): publish docker@0.0.61 885cd16 Revert "fix(appname): encodeURIComponent for app name (#8586)" (#8627) * chore(google): publish google@0.0.22 885cd16 Revert "fix(appname): encodeURIComponent for app name (#8586)" (#8627) * chore(oracle): publish oracle@0.0.10 885cd16 Revert "fix(appname): encodeURIComponent for app name (#8586)" (#8627) * chore(tencentcloud): publish tencentcloud@0.0.7 885cd16 Revert "fix(appname): encodeURIComponent for app name (#8586)" (#8627)
Re-submittal of reverted PR spinnaker#8586
Re-submittal of reverted PR spinnaker#8586
Re-submittal of reverted PR spinnaker#8586
Re-submittal of reverted PR spinnaker#8586
Re-submittal of reverted PR spinnaker#8586
Re-submittal of reverted PR spinnaker#8586
Re-submittal of reverted PR #8586 Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Since the application name is a user supplied value, and that its rules may be changed with plugins, let's encode the url value on network requests.