Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Form] Forward Form::submit(Request) to Form::HandleRequest#16088

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
0mars wants to merge4 commits intosymfony:2.3from0mars:ticket_13291

Conversation

@0mars
Copy link

When the argument of Form::submit is a Request object it forwards to Form::HandleRequest.
Fixes the issue with uploaded files exceeding post_max_size,
and form marked as valid with empty fields.

QA
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#13291
LicenseMIT
Doc PR

…eRequestWhen the argument of Form::submit is a Request object it forwards toForm::HandleRequest.Fixes the issue with uploaded files exceeding post_max_size,and form marked valid with empty fields.
@Tobion
Copy link
Contributor

submit() does not work withRequest onlybind() allows to pass a Request and this is deprecated anyway. See41b0127#diff-0901ceb76939b17cd920bc69aa52d21fR38 Nvm, apparently submit should also support this until 3.0 according to the deprecations, probably to ease the upgrade path so people can just rename bind to submit.

@Tobion
Copy link
Contributor

I think the actual problem is that#11924 fixed the request handlers, but did not fix the legacy appraoch using thehttps://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/Form/Extension/HttpFoundation/EventListener/BindRequestListener.php
But maybe the proposed solution here is really the easiest fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

this should be moved after thethrow new AlreadySubmittedException

Copy link
Contributor

Choose a reason for hiding this comment

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

also you need to skip the following code in this case because the request handler itself also callssubmit. seehttps://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/Form/Extension/HttpFoundation/HttpFoundationRequestHandler.php#L116
so otherwise the form would be submitted twice and an AlreadySubmittedException is thrown.
have you actually tried to use the fix for your issue? it cannot work in its current form AFAIK.

Copy link
Author

Choose a reason for hiding this comment

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

It shouldn't throw the AlreadySubmittedException, because it follows a different execution path inside submit()

1 Form::submit(Request)
1.1 Form::handleRequest(Request)
1.1.1 HttpFoundationRequestHandler::handleRequest(Form, Request)
1.1.1.1 Form::submit(data)

only at: 1.1.1.1 Form::submitted = true

And if we used bind
preBindEvent dispatches (does not submit, so submitted is still false)
bind will call Form::submit(Request) which will follow the same execution path above.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah there is areturn in front of handleRequest. I didn't see that and this is what I thought is missing.

0mars added a commit to 0mars/symfony that referenced this pull requestOct 3, 2015
@fabpotfabpot added the Form labelOct 5, 2015
@fabpotfabpot changed the titleBug #13291 [Form] Forward Form::submit(Request) to Form::HandleRequest[Form] Forward Form::submit(Request) to Form::HandleRequestOct 6, 2015
@fabpot
Copy link
Member

@Tobion Does it get a +1 from you?

@Tobion
Copy link
Contributor

Hm I see one problem. When usingsubmit the form is always submitted. But now that it useshandleRequest instead, the form might not be submitted anymore because the form only gets submitted when the request method is the same as the form config (which defaults to POST). So GET requests with a form might not be submitted anymore which is would be a behavior change.

So I think the only non-breaking solution is to fix thehttps://github.com/symfony/symfony/blob/2.3/src/Symfony/Component/Form/Extension/HttpFoundation/EventListener/BindRequestListener.php according tohttps://github.com/symfony/symfony/blob/2.3/src/Symfony/Component/Form/Extension/HttpFoundation/HttpFoundationRequestHandler.php.

@Tobion
Copy link
Contributor

But then again, this approach is deprecated anyway. So I'm not sure it's really worth to do it.
I'm closing this PR with the current solution due to above reasons.
@0mars If you have the time feel free to create a new PR with the fixed BindRequestListener.

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

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@0mars@Tobion@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp