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

fix(protocol): trailing credential data skipped #191

Merged
merged 1 commit into from
Dec 2, 2023

Conversation

james-d-elliott
Copy link
Member

@james-d-elliott james-d-elliott commented Dec 1, 2023

This fixes an issue where the trailing data in the credential parsing function would be ignored. This also adds some clarity documentation and cleanup to the ParseCredentialCreationResponse function.

Fixes #189

@james-d-elliott james-d-elliott requested a review from a team as a code owner December 1, 2023 22:31
@james-d-elliott
Copy link
Member Author

@mitar can you take a look at this? Pretty sure it's accurate to your suggestions.

@mitar
Copy link
Contributor

mitar commented Dec 1, 2023

There is also ParseCredentialRequestResponseBody which reads JSON?

Otherwise it looks good.

@mitar
Copy link
Contributor

mitar commented Dec 1, 2023

And thanks for fast responses here!

@james-d-elliott
Copy link
Member Author

Ah yep, of course. I'll abstract this away into its own function quickly.

This fixes an issue where the trailing data in the credential parsing functions would be ignored. This also adds some clarity documentation and cleanup to the ParseCredentialCreationResponse and ParseCredentialRequestResponseBody functions.
@james-d-elliott
Copy link
Member Author

james-d-elliott commented Dec 1, 2023

And thanks for fast responses here!

Oh no worries. I appreciate the feedback and help. My time is stretched thin and finding elements like these is rather helpful. Also most of the things you've spotted are relatively easily added/fixed.

@james-d-elliott james-d-elliott merged commit e5a5571 into master Dec 2, 2023
8 checks passed
@james-d-elliott james-d-elliott deleted the fix-json-decoder branch December 2, 2023 00:30
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.

Library uses json.NewDecoder(body).Decode which might not error on trailing extra data
2 participants