Skip to content

Conversation

a1012112796
Copy link
Member

fix #35512

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 22, 2025
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code labels Sep 22, 2025
@techknowlogick
Copy link
Member

CI fail is related:


--- FAIL: TestCreateIssueAttachment (0.49s)
    testlogger.go:61: 2025/09/22 14:12:32 modules/storage/local.go:33:NewLocalStorage() [I] Creating new Local Storage at /home/runner/work/gitea/gitea/tests/gitea-lfs-meta
    testlogger.go:61: 2025/09/22 14:12:32 HTTPRequest [I] router: completed GET /user/login for test-mock:12345, 200 OK in 2.1ms @ auth/auth.go:184(auth.SignIn)
    testlogger.go:61: 2025/09/22 14:12:32 HTTPRequest [I] router: completed POST /user/login for test-mock:12345, 303 See Other in 5.8ms @ auth/auth.go:197(auth.SignInPost)
    testlogger.go:61: 2025/09/22 14:12:32 HTTPRequest [I] router: completed POST /user2/repo1/issues/attachments for test-mock:12345, 500 Internal Server Error in 85.8ms @ repo/attachment.go:25(repo.UploadIssueAttachment)
    attachment_test.go:47: Response:  NewAttachment: attachment size 2147483648 exceed limit 104
        
    attachment_test.go:47: 
        	Error Trace:	/home/runner/work/gitea/gitea/tests/integration/integration_test.go:354
        	            				/home/runner/work/gitea/gitea/tests/integration/integration_test.go:160
        	            				/home/runner/work/gitea/gitea/tests/integration/attachment_test.go:47
        	            				/home/runner/work/gitea/gitea/tests/integration/attachment_test.go:67
        	Error:      	Not equal: 
        	            	expected: 200
        	            	actual  : 500
        	Test:       	TestCreateIssueAttachment
        	Messages:   	Request: POST /user2/repo1/issues/attachments

Also, it's probably better to have a status 413 returned if the size is too large

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Sep 23, 2025
@lunny lunny added the type/bug label Sep 23, 2025
@lunny
Copy link
Member

lunny commented Oct 4, 2025

last call @go-gitea/technical-oversight-committee

Co-authored-by: 6543 <[email protected]>
Signed-off-by: Lunny Xiao <[email protected]>
@lunny lunny added this to the 1.26.0 milestone Oct 6, 2025
@6543
Copy link
Member

6543 commented Oct 7, 2025

@a1012112796 -> a1012112796#13

@6543
Copy link
Member

6543 commented Oct 7, 2025

could push it in myselfe ...

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 7, 2025
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

No, the new changes are wrong.

header.Size can't lie.

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Oct 7, 2025
@6543
Copy link
Member

6543 commented Oct 7, 2025

I try to write a prove of concept

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/v1.25 lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attachment size limit in app.ini not enforced for REST API uploads
6 participants