-
Notifications
You must be signed in to change notification settings - Fork 417
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
Change InterceptorRequest.Body to string #877
Conversation
/assign @wlynch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Storing the body as `json.RawMessage` can lead to loss of information about spaces in the incoming body when the InterceptorRequest is marshaled as JSON before sending it over HTTP. This is because Go's `json.Marshal` will encode `[]byte` as a base64 string and base64 does not preserve spaces. Adding a custom `MarshalJSON` does not help here since `MarshalJSON` will return a `[]byte` that will then get compacted as base64 by `json.Marshal`. This loss of spaces can be problematic for some use cases. For instance, the GitHub payload signature validation requires us to compare using the exact body as it was sent. Otherwise, the validation fails. Note that this will only be a problem when we marshal the InterceptorRequest i.e. when we move the core interceptors out to its own server. Using a string means that the string will be quoted e.g. `{\"foo\": \"bar\"}`. Go should take care of unquoting it during the unmarshaling process. However, intercetpor authors using a different language will have to manually unquote the string by parsing it as a JSON object. Part of tektoncd#271, tektoncd#867 Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wlynch The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Changes
Storing the body as
json.RawMessage
can lead to loss of information aboutspaces in the incoming body when the InterceptorRequest is marshaled as JSON
before sending it over HTTP.
This is because Go's
json.Marshal
will encode[]byte
as a base64 string andbase64 does not preserve spaces. Adding a custom
MarshalJSON
does not helphere since
MarshalJSON
will return a[]byte
that will then get compacted asbase64 by
json.Marshal
.This loss of spaces can be problematic for some use cases. For instance, the
GitHub payload signature validation requires us to compare using the exact body
as it was sent. Otherwise, the validation fails. Note that this will only be a
problem when we marshal the InterceptorRequest i.e. when we move the core
interceptors out to its own server.
Using a string means that the string will be quoted e.g.
{\"foo\": \"bar\"}
.Go should take care of unquoting it during the unmarshaling process. However,
intercetpor authors using a different language will have to manually unquote
the string by parsing it as a JSON object.
Part of #271, #867
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Release Notes