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

Multipart reader regression in go 1.17 #8445

Closed
3 tasks done
Tracked by #8343
Stebalien opened this issue Sep 20, 2021 · 9 comments · Fixed by ipfs/go-ipfs-files#42 or #8488
Closed
3 tasks done
Tracked by #8343

Multipart reader regression in go 1.17 #8445

Stebalien opened this issue Sep 20, 2021 · 9 comments · Fixed by ipfs/go-ipfs-files#42 or #8488
Assignees
Labels
kind/bug A bug in existing code (including security flaws)
Milestone

Comments

@Stebalien
Copy link
Member

Checklist

Installation method

built from source

Version

No response

Config

No response

Description

See the test failure in ipfs/go-ipfs-files#40:

--- FAIL: TestMultipartFiles (0.00s)
    helpers_test.go:122: [2] found end of directory, expected files.Event{kind:0, name:"nested", value:"some content"}
FAIL
FAIL	github.com/ipfs/go-ipfs-files	0.007s
FAIL

This isn't a flake so something is going wrong. I'm not sure if this affects production, but we should figure it out before releasing.

@Stebalien Stebalien added kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization labels Sep 20, 2021
@Stebalien Stebalien added this to the go-ipfs 0.10 milestone Sep 20, 2021
@BigLep
Copy link
Contributor

BigLep commented Sep 20, 2021

Next step:

  1. Confirm this only manifests with go 1.17 and if so release with 1.16.

@Stebalien
Copy link
Member Author

Confirm this only manifests with go 1.17 and if so release with 1.16.

The CI passes with 1.17 but not with 1.16, so I believe we can consider this confirmed. But yeah, we can just not use 1.17 when we release.

@marten-seemann
Copy link
Member

The CI passes with 1.17 but not with 1.16

I believe this should be the other way around: The CI passes with 1.16 but not with 1.17.

Is this a bug in the standard library?

@Stebalien
Copy link
Member Author

Uh, yes, you're right. And it might be a bug, or they might be getting more strict.

@marten-seemann
Copy link
Member

marten-seemann commented Sep 20, 2021

This is probably due to golang/go@784ef4c5313, which fixes golang/go#45789.

@marten-seemann
Copy link
Member

Now the question is, have we been using the multipart reader in an incorrect way, potentially creating a security issue as described in the Go issue, or is this something we should work around by reimplementing the Go 1.16 behavior? @Stebalien?

@Stebalien
Copy link
Member Author

We've been using it exactly as we intended.

@Stebalien
Copy link
Member Author

Fix in ipfs/go-ipfs-files#42

@BigLep BigLep removed the need/triage Needs initial labeling and prioritization label Sep 21, 2021
@BigLep BigLep modified the milestones: go-ipfs 0.10, go-ipfs 0.11 Sep 28, 2021
@lidel
Copy link
Member

lidel commented Oct 5, 2021

Remaining work: tag a new release of https://github.com/ipfs/go-ipfs-files and switch go-ipfs to it in a PR that closes this issue + the one about timestamps.

@lidel lidel reopened this Oct 5, 2021
lidel added a commit that referenced this issue Oct 5, 2021
lidel added a commit that referenced this issue Oct 5, 2021
lidel added a commit that referenced this issue Oct 5, 2021
@guseggert guseggert mentioned this issue Nov 23, 2021
80 tasks
hacdias pushed a commit to ipfs/boxo that referenced this issue Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
No open projects
Archived in project
4 participants