-
Notifications
You must be signed in to change notification settings - Fork 426
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
TriggerTemplate errors with \n character #942
Comments
So @dibyom it looks like newlines that are set as parameters passed in at https://github.com/tektoncd/triggers/blob/master/pkg/resources/create.go#L69 need to be escaped in order to pass as newlines. Unfortunately, it looks like you have to know where that newline string goes in the resolved template. If it shows up in a pipelinev1.ArrayOrString parameter, it needs to be escaped there before it is populated. As you can see in sink_test.go, when it is passed into a label this issue isn't present. Basically, this is about how the triggertemplate is populated with values. You need to know the context of the type that the resolved string is being placed into. |
The problem with finding this context is that triggers replaces the byte arrays in a json.RawMessage, so that context isn't available: https://github.com/tektoncd/triggers/blob/master/pkg/template/resource.go#L135 |
Closes tektoncd#942 This PR resolves the bug by replacing all instances of the \n character in triggerbinding params with \n, thus encoding into JSON as the bytes 92 110 rather than 10 or 13 10 depending on the system. This works in any instance where the parameter is substituted as the JSON encoding will resolve the parameter value after the byte array for the constructed rawMessage is assembled. Therefore, anywhere that the newline character is in one of these parameters, it will not encode appropriately in JSON. This was determined based on the existing tests that use \n in the label fields. Since the JSON encoder already operate successfully with those fields, we can look at the resultant byte array when those tests are run and determine the appropriate input that should be passed to the replace function in resource.go
Scratch that, we can just encode this by escaping the newline anywhere the param is replaced. Submitted PR #953 |
Issues go stale after 90d of inactivity. /lifecycle stale Send feedback to tektoncd/plumbing. |
So, in #953 (comment) I had suggested that we revive and merge #953. I did a bit more digging and it looks like the problem with newline characters is restricted mainly to YAML multilines i.e. either trigger binding values or trigger template param defaults containing YAML multiline strings. If the incoming event body contains a \n, Trigger processing does not fail |
Issues go stale after 90d of inactivity. /lifecycle stale Send feedback to tektoncd/plumbing. |
Stale issues rot after 30d of inactivity. /lifecycle rotten Send feedback to tektoncd/plumbing. |
(This is still a issue we need to fix) |
/priority important-longterm |
Now I am facing a similar problem when I have string array in the JSON body sent to Event Listener |
hi @OSobky do you have a quick example of your use case that is failing? |
Hey @dibyom, Sorry for the late reply. Sure! Basically, I was trying to send a request with an array of strings in the JSON body like the following: However, the event listener generates the following error: Let me know if you need more detailed example |
Issues go stale after 90d of inactivity. /lifecycle stale Send feedback to tektoncd/plumbing. |
Expected Behavior
Users should be able to use a Trigger with binding or template param values that contain the newline
\n
character.Actual Behavior
TriggerTemplate errors out. I tried a few cases cases -- 1) TriggerTemplate with a default param that contains a newline, 2) with the annotation
triggers.tekton.dev/old-escape-quotes: "true"
added, and 3) with a binding value that contains the new line param.Steps to Reproduce the Problem
Additional Info
Related:
TEP on escaping quotes: Escaping quotes in TriggerTemplates. community#321
earlier bug report: TriggerBinding doesn't support multiline string param? #772
Tekton Triggers version: v0.11.2
Error logs
The text was updated successfully, but these errors were encountered: