Skip to content

make sure to avoid closing the input reader#2137

Merged
harshavardhana merged 1 commit intominio:masterfrom
harshavardhana:avoid-closer
Jul 21, 2025
Merged

make sure to avoid closing the input reader#2137
harshavardhana merged 1 commit intominio:masterfrom
harshavardhana:avoid-closer

Conversation

@harshavardhana
Copy link
Copy Markdown
Member

No description provided.

@harshavardhana harshavardhana merged commit bd91926 into minio:master Jul 21, 2025
5 checks passed
@harshavardhana harshavardhana deleted the avoid-closer branch July 21, 2025 06:43
Comment thread hook-reader.go
@@ -84,7 +84,7 @@ func (hr *hookReader) Read(b []byte) (n int, err error) {
// reports the data read from the source to the hook.
func newHook(source, hook io.Reader) io.Reader {
if hook == nil {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The if could be dropped:

 return &hookReader{
	source: source,
	hook:   hook,
}

Should be enough

saschazepter pushed a commit to saschazepter/forgejo that referenced this pull request Aug 10, 2025
Fixes #8529, supersedes #8597
Reverts the workaround of #8541 (while keeping the test) and ignore Close error (like #8527 did).

minio/minio-go#2137 makes the test pass without the `io.NopCloser`:

```
docker run --rm -e MINIO_DOMAIN=minio -e MINIO_ROOT_USER=123456 -e MINIO_ROOT_PASSWORD=12345678 -p 9000:9000  data.forgejo.org/oci/bitnami/minio:2024.8.17
```

```
TEST_MINIO_ENDPOINT=localhost:9000  go test -v -run ^TestMinioStorageIterator$ ./modules/storage
```

Wrapping the `io.Reader` in an `io.NopCloser` also hides all other methods (like `WriteTo` or `Seek`), so is not ideal.

As a further "line of defense", the Close error is ignored (if saveAsPackageBlob was succesful, we don't really care if Close fails).

Co-authored-by: Renovate Bot <[email protected]>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8816
Reviewed-by: Gusted <[email protected]>
Co-authored-by: oliverpool <[email protected]>
Co-committed-by: oliverpool <[email protected]>
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

Successfully merging this pull request may close these issues.

2 participants