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

Introduce AbpRequestSizeLimitMiddleware #12358

Merged
merged 2 commits into from
Apr 27, 2022
Merged

Introduce AbpRequestSizeLimitMiddleware #12358

merged 2 commits into from
Apr 27, 2022

Conversation

maliming
Copy link
Member

Resolve #12348

if (MultiTenancyConsts.IsEnabled)
{
    // Before the multi-tenancy middleware.
    app.UseAbpRequestSizeLimit();
    app.UseMultiTenancy();
}

public class FileController : AbpControllerBase, IRemoteService
{
    [HttpPost]
    [AbpRequestSizeLimit(5)]
    public Task<IActionResult> Upload(IFormFile file)
    {
        return Task.FromResult<IActionResult>(Content("test"));
    }
}

@maliming maliming requested a review from hikalkan April 26, 2022 05:46
@hikalkan hikalkan merged commit 2473aeb into dev Apr 27, 2022
@hikalkan hikalkan deleted the RequestSizeLimit branch April 27, 2022 12:26
@hikalkan
Copy link
Member

I have a few questions:

  1. Why we are putting this into if MultiTenancyConsts.IsEnabled? Isn't it needed if multi-tenancy disabled? If so, will AbpRequestSizeLimit attribute will work when multi-tenancy disabled? If not, that's a big problem. Assume that I've created a module with using this attribute. Any application that is not multi-tenant won't use it. Right?

  2. Why is the standard RequestSizeLimit not working. Can you please write the reason.

  3. Where should we document this?

@maliming
Copy link
Member Author

The multi-tenant middleware(FormTenantResolveContributor) will read the request body. After that RequestSizeLimit won't work because the request stream has already been read.

I will update the multi-tenant document.

@JadynWong
Copy link
Contributor

Can we use RequestSizeLimitAttribute directly if RequestSizeLimit is handled only when multi-tenancy is enabled?

By default aspnetcore handles RequestSizeLimit, abp handles it when multi-tenant is enabled.

image

@maliming
Copy link
Member Author

Can we use RequestSizeLimitAttribute directly if RequestSizeLimit is handled only when multi-tenancy is enabled?

By default aspnetcore handles RequestSizeLimit, abp handles it when multi-tenant is enabled.

image

Looks good.

@hikalkan
Copy link
Member

@JadynWong that is much better than introducing a new attribute that can't work in a single-tenant environment. I agree. But, we may also resolve the issue by fixing the problem with multi-tenancy.

@hikalkan
Copy link
Member

hikalkan commented Apr 27, 2022

I think we can remove FormTenantResolveContributor from the contributors list by default and remove from the multi-tenancy documentation. I believe no one uses it. We deprecate it and remove from the framework in 6.0.
So, we can rollback this PR in that case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce AbpRequestSizeLimitAttribute
3 participants