Percentage symbol % caused extra characters set in variable

[Yiming-Jia]

Please check our current Issues to see if someone already reported this https://github.com/Microsoft/azure-pipelines-task-lib/issues
No.

Environment

azure-pipelines-task-lib version: "2.9.3"

Issue Description

When customer tried to set variable with value myPassword%, the actual value became myPassword%25.

Expected behaviour

myPassword% instead of myPassword%25

Actual behaviour

myPassword%25

Steps to reproduce

This issue happened under task Azure App Configuration. And after testing, we confirm this is a bug from azure-pipelines-task-lib. Original issue created under Azure App Configuration GitHub issue: Azure/AppConfiguration#313

#Please confirm if this is bug and an ETA for the fix.

[damccorm]

This is expected. Because \r and \n are reserved values, we need a way of escaping them. To do so we reserve % as a special character for encoding (following html encoding practices), and we escape % with %25 (again following html encoding practices). You can see the escaping logic here.

Note that if you use the task-lib to get the value we handle the escaping for you. If that's not acceptable for your use case, I'd recommend emitting your own ## command using stdout instead of this library's setVariable function.

[Yiming-Jia]

@damccorm , I'm confused here. I know URI encoding. Could you justify why do you do URI encoding for variable value? From your source code, it seems that you only picked few your interested character to do escaping.

Also, there should be a way for us to escape characters that you reserved, like \.

[Yiming-Jia]

Also, could you provide a link for raw command interface, like the one you mentioned above ##. Just a friendly suggestion: I think it's better to close any issues after you address customers questions.

[damccorm]

it seems that you only picked few your interested character to do escaping.

Yeah, there's only a few values that need to be protected. Those are \r and \n since we use those to detect the end of a line (and thus the end of a ## command). We also escape % since that is the special character that is used for escaping the other characters. We chose URI encoding (as opposed to a different scheme) because its generally efficient and standardized, but we don't need the whole set of uri encoding since most of those characters don't need to be reserved in our model.

Also, there should be a way for us to escape characters that you reserved, like .

\ is not reserved. The only reserved values for data are \r, \n, and % and we escape those automatically. ] and ; are also reserved for values appearing inside the ## command, we escape those automatically as well.

So in ##vso[task.setvariable variable={A}]{B} for example, {A} will have \r, \n, %, ], and ; escaped automatically and {B} will have \r, \n, and % escaped

[damccorm]

could you provide a link for raw command interface, like the one you mentioned above ##

https://github.com/microsoft/azure-pipelines-tasks/blob/master/docs/authoring/commands.md

[Yiming-Jia]

Hello @damccorm,

Yes, I understand you're escaping % \r and \n. Will it cause a vso command failure if I just set \r or \n as environment variable.

Also, in our case, we set environment variables by using task library, and let customer consumer later in their build and deployment. However, customer will fetch variables as environment variables instead of call task library. Is there a way to let customer bypass the escape?

[Yiming-Jia]

Hello @damccorm,

Any updates on my previous question? As we think if you reserve some characters, you must provider caller a way to escape them. Please also reopen the issue. Also, we don't really want to write a separate method for setVariable as we have to maintain it and follow the contract with DevOps logging command by mimicking behavior of azure-pipeline-task-lib

[damccorm]

Is there a way to let customer bypass the escape?

If you want to not get escaping, we recommend just emitting your own ## commands directly rather than using this library for it.

[zachariahcox]

If I'm reading this correctly, it looks like @Yiming-Jia, you're trying to set a variable using the utility library for a customer who will NOT use the utility library to read it? The basic problem is that our tool assumes usage for both reading and writing and you're asking for a mode to turn off escaping in your case? Is that right?

If so, I can see why that would be sort of surprising, but I think @damccorm did a good job explaining why it's the case -- basically this library is generally designed to be a convenience and most customers will either get and set variables using the library or use it for neither. Additionally, some customers will set a second ##vso command as the value of a variable. For these folks, using the tool is simple and effective: we will automatically escape-out the bare minimum of the characters required for us to function safely.

For your use case, I would second the recommendation to just use a console.log("") for your variable that needs to be set in this way. Your use case doesn't seem to fit the general intent of the tool, and adding the noDataEscape option to every function seems like going overboard, right?
You can certainly still depend on the task-lib for most other functionality and can still default to using it for setting variables in any of your other cases that do not involve both reserved characters and setting from the tool / reading from env.

If this doesn't seem like the right approach to you, please feel free to reach out to me directly to elaborate or open a convincing PR.
Thanks!

[DerGary]

wow this is really inconvenient. We always used the library to set a variable and then used in in subsequent tasks as parameters with $(VariableName). Until it broke our builds when a % sign is used. This should be at least declared in the documentation. I think a lot of customers use the api like we do and provide variables for subsequent tasks as parameters

[jimmyca15]

It seems like devops has a special notion for these variables that are created through setVariable, and devops just happens to use environment variables as the storage for these variables. I see this as problematic. It encourages a usage pattern to consume the variables directly through environment variables, or as @DerGary mentioned his consumption pattern is. The problem is that any of these users are going to get bitten as soon as they use a % sign.

I would suggest that if DevOps needs to special encode these variables and store them somehow that they store them in a way where they can't be relied on to be consumed from environment variables. As an example, md5 hash the variable name and store the variable with the hash as the key. It shows that it's not meant to be consumed by name directly from environment variable.

Then, task developers who want their variables to be propagated through the task and accessible via environment variable and task variable can set them in both ways which will actually store two separate environment variables.

[jessehouwing]

The docs even spell out how to reference variables from the environment.

https://docs.microsoft.com/en-us/azure/devops/pipelines/process/variables