Skip to content

test: add test for PresignedPostPolicy with empty fileName - #2119

Merged
harshavardhana merged 4 commits into
minio:masterfrom
jiuker:test-add-test-for-PresignedPostPolicy-with-empty-fileName
Jun 23, 2025
Merged

test: add test for PresignedPostPolicy with empty fileName#2119
harshavardhana merged 4 commits into
minio:masterfrom
jiuker:test-add-test-for-PresignedPostPolicy-with-empty-fileName

Conversation

@jiuker

@jiuker jiuker commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

test: add test for PresignedPostPolicy with empty fileName
fix minio/minio#21323 (review)

We should add a test for this in mint.
jiuker
test: add test for PresignedPostPolicy with empty fileName
fed4256 
test: add test for PresignedPostPolicy with empty fileName
Copilot AI reviewed Jun 4, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a new functional test to verify behavior when the form file name is empty in a presigned POST policy, and wires it into the test suite.

  • Introduce testPresignedPostPolicyEmptyFileName() covering empty filename error case
  • Invoke the new test in main()
  • No other functional changes
Comment thread functional_tests.go
logError(testName, function, args, startTime, "", "File open failed", err)
return
}
w, err := writer.CreateFormFile("", filePath)

Copilot AI Jun 4, 2025

Copy link

Choose a reason for hiding this comment

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

The arguments to CreateFormFile are swapped: the first parameter is the form field name and the second is the filename. To test an empty filename, use writer.CreateFormFile("file", "") (or swap the args) instead of an empty field name.
Copilot uses AI. Check for mistakes.
Comment thread functional_tests.go
}

// testPresignedPostPolicyEmptyFileName tests that an empty file name in the presigned post policy
func testPresignedPostPolicyEmptyFileName() {

Copilot AI Jun 4, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] This new test duplicates a large portion of testPresignedPostPolicy and testPresignedPostPolicyWrongFile. Consider extracting common setup (client initialization, bucket creation, data reader, policy builder) into a helper to reduce duplication and improve readability.
Copilot uses AI. Check for mistakes.
@jiuker

jiuker commented Jun 4, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@harshavardhana Looks like need the latest minio which have the fix to test https://github.com/minio/minio-go/actions/runs/15437786953/job/43448139531?pr=2119

{"time":"2025-06-04T08:49:05.758503756Z","level":"INFO","name":"minio-go: testPresignedPostPolicy","duration":22,"function":"PresignedPostPolicy(policy)","args":{"policy":"{\"expiration\":\"2025-06-14T08:49:05.743Z\",\"conditions\":[[\"eq\",\"$bucket\",\"minio-go-test-lbrqmi4xsygunklr\"],[\"eq\",\"$key\",\"sb4h6ajpod9bijx1c60olk1rqylcwv\"],[\"eq\",\"$Content-Type\",\"binary/octet-stream\"],[\"eq\",\"$x-amz-meta-userrwz5oksy0daruerus9cz3qdlgk\",\"p2ihru65wooasbucen69enghxvz6sa\"],[\"eq\",\"$Content-Encoding\",\"gzip\"],[\"eq\",\"$x-amz-checksum-algorithm\",\"CRC32C\"],[\"eq\",\"$x-amz-checksum-crc32c\",\"aHnJMw==\"],[\"content-length-range\", 10, 1048576]]}","url":"https://localhost:9000/minio-go-test-lbrqmi4xsygunklr/"},"status":"PASS"}
{"time":"2025-06-04T08:49:05.778829608Z","level":"INFO","name":"minio-go: testPresignedPostPolicyWrongFile","duration":13,"function":"PresignedPostPolicy(policy)","args":{"policy":"{\"expiration\":\"2025-06-14T08:49:05.772Z\",\"conditions\":[[\"eq\",\"$bucket\",\"minio-go-test-w4ombnn9vsb5ychf\"],[\"eq\",\"$key\",\"eplpx1b9910iy51jnb155nvwfxs5a9\"],[\"eq\",\"$Content-Type\",\"binary/octet-stream\"],[\"eq\",\"$x-amz-meta-user422zdocvnpvq0xeqa2tzzt02r0\",\"6ipbcbif32txypqrwdxxkrvvcnxs9t\"],[\"eq\",\"$x-amz-checksum-algorithm\",\"CRC32C\"],[\"eq\",\"$x-amz-checksum-crc32c\",\"8TDyHg==\"],[\"content-length-range\", 10, 1048576]]}","url":"https://localhost:9000/minio-go-test-w4ombnn9vsb5ychf/"},"status":"PASS"}
ERRO: panic: "POST /minio-go-test-eccrd4n9lvsokpql/": runtime error: invalid memory address or nil pointer dereference
goroutine 150256 [running]:
runtime/debug.Stack()
	runtime/debug/stack.go:26 +0x5e
github.com/minio/minio/cmd.serverMain.func10.setCriticalErrorHandler.2.1()
	github.com/minio/minio/cmd/generic-handlers.go:583 +0x99
panic({0x306c540?, 0x6cfa2e0?})
	runtime/panic.go:792 +0x132
github.com/minio/minio/internal/etag.(*Reader).Read(0xc002fc14c0, {0xc00d6ba000, 0x43b987?, 0x200000})
	github.com/minio/minio/internal/etag/reader.go:129 +0x36
github.com/minio/minio/internal/hash.(*Reader).Read(0xc010c0e0d0, {0xc00d6ba000, 0xc00d343f78?, 0x200000})
	github.com/minio/minio/internal/hash/reader.go:251 +0x42
io.ReadAtLeast({0x4f10500, 0xc010c0e0d0}, {0xc00d6ba000, 0x100000, 0x200000}, 0x100000)
	io/io.go:335 +0x91
io.ReadFull(...)
@klauspost

klauspost commented Jun 12, 2025

Copy link
Copy Markdown
Contributor

@jiuker What you posted looks more like a bug than a test failure to me. Consider failing rather than crashing.
@jiuker

jiuker commented Jun 12, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@jiuker What you posted looks more like a bug than a test failure to me. Consider failing rather than crashing.

A bug fixed at minio main branch.
@klauspost

klauspost commented Jun 13, 2025

Copy link
Copy Markdown
Contributor

@jiuker Which one?
@jiuker

jiuker commented Jun 13, 2025

Copy link
Copy Markdown
Contributor Author

@jiuker Which one?

minio/minio#21323 this one
@klauspost

klauspost commented Jun 13, 2025

Copy link
Copy Markdown
Contributor

@jiuker How can a remote package cause a runtime error: invalid memory address or nil pointer dereference in a local test?
@jiuker

jiuker commented Jun 13, 2025

Copy link
Copy Markdown
Contributor Author

@jiuker How can a remote package cause a runtime error: invalid memory address or nil pointer dereference in a local test?

Should be remote minio have the bug code.
@klauspost

klauspost commented Jun 16, 2025

Copy link
Copy Markdown
Contributor

@jiuker The test in this package should not panic, just because it doesn't get the response it expects. It should log the error and move on.
@jiuker

jiuker commented Jun 17, 2025

Copy link
Copy Markdown
Contributor Author

@jiuker The test in this package should not panic, just because it doesn't get the response it expects. It should log the error and move on._这个包中_的测试不应该仅仅因为没有得到预期的响应而死机。它应该记录错误并继续前进。

Panic happend at minio code, not this package.
@harshavardhana

harshavardhana commented Jun 17, 2025

Copy link
Copy Markdown
Member

Panic happend at minio code, not this package.

are you sending a fix?
@jiuker

jiuker commented Jun 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Panic happend at minio code, not this package.

are you sending a fix?

Already merged minio/minio#21323 @harshavardhana

minio-go/.github/workflows/go.yml

Lines 43 to 51 in 1744ff2

run: |
sudo apt update -y
sudo apt install devscripts -y
wget -O /tmp/minio https://dl.minio.io/server/minio/release/linux-amd64/minio
chmod +x /tmp/minio
mkdir -p /tmp/certs-dir
cp testcerts/* /tmp/certs-dir
/tmp/minio server --quiet -S /tmp/certs-dir /tmp/fs{1...4} &
make

This release don't have that fix
@harshavardhana
harshavardhana merged commit 8eacd80 into minio:master Jun 23, 2025
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants

@jiuker @klauspost @harshavardhana