Skip to content

TestHTTPClientSanitizeJSONControlCharactersC0 fails with go-1.22 #8668

Description

@mikelolasagasti

Describe the bug

Test TestHTTPClientSanitizeJSONControlCharactersC0 in api/http_client_test.go fails with go-1.22 as the commit golang/go@2763146 changes the enconding of \b and \f escape sequences in JSON strings.

Steps to reproduce the behavior

  1. go test ./api/ > test.out
  2. cat -A test.out

Expected vs actual behavior

--- FAIL: TestHTTPClientSanitizeJSONControlCharactersC0 (0.00s)$
    http_client_test.go:225: $
        ^IError Trace:^I/cli/api/http_client_test.go:225$
        ^IError:      ^INot equal: $
        ^I            ^Iexpected: "1^A 2^B 3^C 4^D 5^E 6^F 7^G 8^H 9\t A\r\n B\v C^L D\r\n E^N F^O"$
        ^I            ^Iactual  : "1^A 2^B 3^C 4^D 5^E 6^F 7^G 8\b 9\t A\r\n B\v C\f D\r\n E^N F^O"$

This change fixes the test, but is not backwards compatible.

diff --git a/api/http_client_test.go b/api/http_client_test.go
index dca032e6..ce20a268 100644
--- a/api/http_client_test.go
+++ b/api/http_client_test.go
@@ -222,7 +222,7 @@ func TestHTTPClientSanitizeJSONControlCharactersC0(t *testing.T) {
        err = json.Unmarshal(body, &issue)
        require.NoError(t, err)
        assert.Equal(t, "^[[31mRed Title^[[0m", issue.Title)
-       assert.Equal(t, "1^A 2^B 3^C 4^D 5^E 6^F 7^G 8^H 9\t A\r\n B\v C^L D\r\n E^N F^O", issue.Body)
+       assert.Equal(t, "1^A 2^B 3^C 4^D 5^E 6^F 7^G 8\b 9\t A\r\n B\v C\f D\r\n E^N F^O", issue.Body)
        assert.Equal(t, "10^P 11^Q 12^R 13^S 14^T 15^U 16^V 17^W 18^X 19^Y 1A^Z 1B^[ 1C^\\ 1D^] 1E^^ 1F^_", issue.Author.Name)
        assert.Equal(t, "monalisa \\u00^[", issue.Author.Login)
        assert.Equal(t, "Escaped ^[ \\^[ \\^[ \\\\^[", issue.ActiveLockReason)

As the change is not backward compatible, unsure how to fix this.

Activity

  1. andyfeller commented on Feb 14, 2024

    @andyfeller
    Contributor

    @mikelolasagasti : thanks for opening up this issue! 🤗

    This is good to know for when we upgrade from Go 1.21 to 1.22 or newer; I will be honest I was unfamiliar with \b backspace and \f form feed use in JSON. With 1.22 being fairly recent, we haven't talked about upgrading, so I'll make sure this is on @williammartin radar.

  2. added
    tech-debtA chore that addresses technical debt
    and removed
    bugSomething isn't working
    on Feb 14, 2024
  3. heaths commented on Feb 15, 2024

    @heaths
    Contributor

    As a workaround, you can set GOTOOLCHAIN=go1.21+auto e.g.,

    GOTOOLCHAIN=go1.21+auto go test ./...

    It will download the latest go1.21 toolchain - currently 1.21.5 - and run tests using that.

  4. mikelolasagasti commented on Feb 15, 2024

    @mikelolasagasti
    ContributorAuthor

    I detected the issue in Fedora 40 as it will ship go-1.22 and found the error while packaging gh.

    I reported here to avoid you wasting time finding where the issue comes from. No workaround required in my case.

  5. williammartin commented on Apr 15, 2024

    @williammartin
    Member

    This was resolved in #8836, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    tech-debtA chore that addresses technical debt

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions