MicrosoftPremier / VstsExtensions

Documentation and issue tracking for Microsoft Premier Services Visual Studio Team Services Extensions
MIT License
59 stars 14 forks source link

CreateWorkItem task escapes " character without checking if already \" #55

Closed inverted360 closed 4 years ago

inverted360 commented 4 years ago

Found a niche bug when setting description field using CreateWorkItem task - If the text I included in Additional Fields > Description > Value was already escaped - example \"http://www.url.com\" then the task logic appends an additional \ to the " character resulting in \\"http://www.url.com\\" in my scenario it resulted in the ADO path concatenated with desired link and was broken.

suggest you update the validation code for description field to use something like -replace '(?<!\\)"','\"'

ReneSchumacher commented 4 years ago

Hi @inverted360,

thank you for reporting this! We'll look into the issue and will release a fix as quick as possible (most probably within this week).

René

ReneSchumacher commented 4 years ago

Hi @inverted360,

sorry for the delay - December has been busier than expected. I have thought about this issue and I don't think we should change the behavior. Our code ensures that every character that needs to be escaped is escaped properly. It cannot really know if it "needs" to escape something or not. Otherwise we wouldn't be able to put escaped characters into the work item.

Is there a reason for using already escaped values in the task?

René

inverted360 commented 4 years ago

Our code ensures that every character that needs to be escaped is escaped properly

I agree with your statement - my feedback was to improve the code so that if a double quote was already escaped, then code behavior would not add another backslash to the string. In my experience, adding another backslash resulted in incorrect parsing during execution. You're welcome to switch the WIT from bug to 'feature request.'

ReneSchumacher commented 4 years ago

Thanks for your feedback. I do see your point. Yet, the task set the values in the work item exactly as they are specified. In other words: if you put escaped characters in the values, those escaped characters end up in the work item field, which is what most people probably expect.

As I said, the task cannot know your intention. If you have something like \" in your value, it could mean that you want that string to occur as is, or that you already escaped it and just want the double quotes. We added the escaping logic to ensure that the task wouldn't run into an error when a user did not carefully escape every character as needed. That is why the task escapes all characters that need to be escaped and your value \"http://www.url.com\" actually becomes \\\"http://www.url.com\\\". The only other logical option I see would be to fully remove the escaping and force users to escape themselves.

What parsing error did occur in your case? Did that occur when you clicked the link? Or during build execution?

ReneSchumacher commented 4 years ago

Since there hasn't been any more information about the parsing issue, I'm closing the issue now. Feel free to open a new one if you run into problems with the Create Work Item extension.