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

Properly urlescape password in tests hook and Jenkins pipeline #468

Merged
merged 3 commits into from
Apr 22, 2022

Conversation

eiri
Copy link
Member

@eiri eiri commented Apr 21, 2022

Checklist

  • Tick to sign-off your agreement to the Developer Certificate of Origin (DCO) 1.1
  • Added tests for code changes or test/build only changes
  • Updated the change log file (CHANGES.md|CHANGELOG.md) or test/build only changes
  • Completed the PR template below:

Description

This PR properly encodes password in Jenkins pipeline as required by our own README and decodes it in test's hook, since node's new URL doesn't do that. This solves an issue with test password that might contain URL components.

Also this PR bumps eslint and eslint-config-standard to unlock its cross deps, that break npm ci

Approach

Schema & API Changes

Security and Privacy

Testing

Monitoring and Logging

Update eslint-config-standard and eslint to avoid unresolved deps look
Properlt urlescape password in Jenkins pipeline
and decode it in test hook while setting up test instance
@eiri eiri marked this pull request as ready for review April 22, 2022 01:57
@eiri eiri changed the title fix tests hook Properly urlescape password in tests hook and Jenkins pipeline Apr 22, 2022
Jenkinsfile Show resolved Hide resolved
@vmatyus
Copy link
Contributor

vmatyus commented Apr 22, 2022

since node's new URL doesn't do that.

The opposite, is true new URL does encoding in hook.js.
But as I see the DB comparison fails even after password decoding.
Might this is the problem, an encoded url should be stored in env var to be a match with the backed-up DB.

Here is a small node snippet:

> url = "http://user:p@ss@localhost"
'http://user:p@ss@localhost'
> new URL(url)
URL {
  href: 'http://user:p%40ss@localhost/',
  origin: 'http://localhost',
  protocol: 'http:',
  username: 'user',
  password: 'p%40ss',
  host: 'localhost',
  hostname: 'localhost',
  port: '',
  pathname: '/',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}

@eiri
Copy link
Member Author

eiri commented Apr 22, 2022

@vmatyusGitHub yes, this is exactly what I said, node's doesn't do decoding of urlencoded password, so this need to be done in hook

@eiri eiri merged commit f5bf4e7 into master Apr 22, 2022
@eiri eiri deleted the fix-tests-hook branch April 22, 2022 14:30
@ricellis ricellis added this to the 2.next milestone Apr 29, 2022
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.

3 participants