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

JsonException with Release 1.1.0 when using VSS_NUGET_EXTERNAL_FEED_ENDPOINTS #484

Closed
sailro opened this issue Mar 6, 2024 · 4 comments
Closed

Comments

@sailro
Copy link
Member

sailro commented Mar 6, 2024

Hello,

Starting with 1.1.0, the credential provider is now using System.Text.Json instead of NewtonSoft.Json.

But this can lead to breaking changes, as in the example below:

We were using docker images like this:

ARG nugetFeedEndpointCredentials
ENV VSS_NUGET_EXTERNAL_FEED_ENDPOINTS=$nugetFeedEndpointCredentials
RUN \
wget --quiet https://raw.githubusercontent.com/Microsoft/artifacts-credprovider/master/helpers/installcredprovider.sh \
&& chmod +x installcredprovider.sh \
&& ./installcredprovider.sh 

And using them in our AZDO build pipeline (see how we are using single quotes for endpointCredentials):

      - script: |
          docker [...] --build-arg nugetFeedEndpointCredentials="{'endpointCredentials':[{'endpoint':'https://foo/bar/_packaging/baz','username':'optional','password':'$(System.AccessToken)'}]}"

As soon as the 1.1.0 was published we hit:

[NuGet Manager] [Error]     [CredentialProvider]Failed to acquire session token: System.Text.Json.JsonException: ''' is an invalid start of a property name. Expected a '"'. Path: $ | LineNumber: 0 | BytePositionInLine: 1.

Indeed single quotes around string values are not supported by design in System.Text.Json.

System.Text.Json only accepts property names and string values in double quotes because that format is required
 by the RFC 8259 specification and is the only format considered valid JSON.

A value enclosed in single quotes results in a `JsonException` with the message: ''' is an invalid start of a value.

Reference: https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/migrate-from-newtonsoft?pivots=dotnet-9-0

Of course we can fix this by using double-quotes going forward, but this change will probably break several users.

As an immediate workaround, before analyzing the issue we used:

ENV AZURE_ARTIFACTS_CREDENTIAL_PROVIDER_VERSION=v1.0.9

in our docker files to rollback to 1.0.9.

Regards
Seb

@sailro sailro changed the title JsonException with Release 1.10.0 when using VSS_NUGET_EXTERNAL_FEED_ENDPOINTS JsonException with Release 1.1.0 when using VSS_NUGET_EXTERNAL_FEED_ENDPOINTS Mar 6, 2024
@MartinK-eu
Copy link

Thank you for the tip, the workaround solved the issue.
It's frustrating that we have to modify all of our Dockerfiles now.

@axelgenus
Copy link

We were affected by this too: we pass the value by means of a variable group so we had to change only the single variable.

embetten added a commit that referenced this issue Mar 6, 2024
- Reverting system.text.json #393 PR due to #484.
- Adding warning message for single quotes in endpoint message.
- Did not change swix/swr dependencies.

---------

Co-authored-by: Jonathan Myers <11822817+jmyersmsft@users.noreply.github.com>
narasamdya pushed a commit to microsoft/BuildXL that referenced this issue Mar 6, 2024
…ange

We are affected by a change in the JSON libraries of the credential provider - microsoft/artifacts-credprovider#484

Related work items: #2156510
@embetten
Copy link
Contributor

embetten commented Mar 6, 2024

Apologies for the breaking change. A new 1.1.1 release to revert has been created and set as latest.

We would like to use System.Text.Json in future versions and env variables with single quotes will need to be updated.

@embetten embetten closed this as completed Mar 6, 2024
@MartinK-eu
Copy link

Apologies for the breaking change. A new 1.1.1 release to revert has been created and set as latest.

We would like to use System.Text.Json in future versions and env variables with single quotes will need to be updated.

Thank you for the quick reaction, the grace period is appreciated.

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

No branches or pull requests

4 participants