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

Set correct pin-value format in tests #376

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

sarroutbi
Copy link
Contributor

@sarroutbi sarroutbi commented Apr 11, 2024

Description

As described in #360, correct format for pin-value option in URI is for it to be after '?', rather than as an additional URI path. This change sets the values appropriately in tests to identify possible parser issues

Fixes #360

Checklist

  • Test suite updated for functionality tests

Reviewer's checklist:

  • Any issues marked for closing are addressed
  • There is a test suite reasonably covering new functionality or modifications
  • Code conform to coding style that today cannot yet be enforced via the check style test
  • Commits have short titles and sensible commit messages

As described in latchset#360, correct format for pin-value
option in URI is for it to be after '?', rather than
as an additional URI path. This change sets the
values appropriately in tests to identify possible
parser issues

Signed-off-by: Sergio Arroutbi <[email protected]>
@sarroutbi sarroutbi marked this pull request as ready for review April 11, 2024 11:07
@simo5
Copy link
Member

simo5 commented Apr 11, 2024

Thanks, looks good!

@simo5 simo5 merged commit 6920c24 into latchset:main Apr 11, 2024
27 checks passed
@Jakuje
Copy link
Contributor

Jakuje commented Apr 11, 2024

Should we also update the parser to not accept the "bad" values or at least warn the user he did something "wrong"?

@simo5
Copy link
Member

simo5 commented Apr 11, 2024

No, we already have this out there, we'd unnecessarily break users.
We should just make sure to change docs to use the correct format, but allowing pin to be passed in the path is fine, it doesn't cause any obvious issues.

@simo5
Copy link
Member

simo5 commented Apr 11, 2024

To be clear,
I would not reject a PR to emit a warning in the debug/openssl log for pin-value and/or path values in the incorrect place, but it is not a priority.

@sarroutbi sarroutbi deleted the 202404111255-fix-pin-values branch April 23, 2024 08:55
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.

the pkcs11-provider takes pin-value query option from the PKCS#11 URI path
3 participants