[Bug]: Error message on uploading Logo shows 100MB instead of 1MB

[chandannkumar]

Is there an existing issue for this?

• [X] I have searched the existing issues

Description

Since uploading logo limit size is 1MB, but on it throws error message saying Payload too large: File size cannot exceed 100MB instead of 1MB as uploading file more than 1mb is not allowed

Steps To Reproduce

1. create a application with multiple page
2. Go to App Settings> navigation> Upload Logo
3. Try to upload valid image larger than 1MB
4. Observe the error says 100MB instead of 1MB

Public Sample App

No response

Environment

Deploy Preview

Issue video log

No response

Version

Cloud

[harshitpandey0426]

This message comes from messages.ts file. Which is

Payload too large. File size cannot exceed ${maxFileSize}MB.

and maxFileSize comes from ApiUtils.ts. Which is

createMessage(ERROR_413, 100)

Changed this maxFileSize from 100 to 1

I have raised a PR for this bug, please have a look. @somangshu @dhruvikn

[harshitpandey0426]

@somangshu @dhruvikn How to get maintainers approval to run tests on my PR?

[somangshu]

Thanks for raising the PR @harshitpandey0426, Ill take it forward from here. Ill keep you posted on the PR is anything is needed!

[dhruvikn]

Hey @harshitpandey0426, thanks for taking a look at the issue. I saw your PR and disagree with the changes you have made. We don't want to change the apiFailureResponseInterceptor's limit to 1 MB. This function is called with all APIs and we have deliberately set the limit to 100 MB to avoid any large file uploads.

Coming back to the issue at hand, we need to triage why this is happening. It could be due to anything such as -

1. The frontend validation of the logo upload failed and allowed a file size bigger than 1 MB.
2. The backend validation failed for the same reason.
3. Or anything else.

If you'd still like to contribute, I would request you to triage the issue first and post your findings here. Based on that, we can come up with a plan and include all the relevant people (backend and/or product folks) in our conversation.

For now, I'm requesting changes to your PR to avoid merging.

cc: @somangshu, do you think we should close the PR instead?

[harshitpandey0426]

Hey @dhruvikn, Thanks for pointing this out. I will have another look at it and share if I find anything relevant

[harshitpandey0426]

For some reason I dont see Admin Settings in local setup. Any suggestion how can I enable this?

[dhruvikn]

You will need a business edition license for that.

May I ask why you need admin settings in your local setup and how are they related to this issue?

[harshitpandey0426]

I was going through other flows to add logo and check validation. One other flow which I found was admin Settings -> Branding -> logo. The Bug mentioned here is not reproducible any more both in local and production PS:

Screenshot for justification

@dhruvikn please lmk If I am missing out anything

[dhruvikn]

The issue is still reproducible in release. I have triaged this issue. My findings are -

1. The FE validation is failing. It was set to 2 MB on the FE when BE accepts a max size of 1 MB.
2. The BE validation is also failing to an extent. It should have thrown the error that the file size cannot exceed 1 MB, but instead, it threw the error mentioned in the issue's description. @NilanshBansal can you please check once? I'm not sure but I think it could be related to Line 97-107 of appsmith/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AssetServiceCEImpl.java

@harshitpandey0426, do you want to pick the FE change up? It should be a teeny-tiny PR.

[dhruvikn]

Also, marking this as low priority since the feature isn't available to public at the moment.

[harshitpandey0426]

@dhruvikn Sure I will check this

[dhruvikn]

Awesome, assigning the issue to you. Let me know if you need any help. Thanks in advance, @harshitpandey0426!

[harshitpandey0426]

@dhruvikn In release I dont see "show logo" toggle. I can see this toggle in production though

[dhruvikn]

That's because the feature is hidden behind a feature flag. If you want to test this, you'll need to setup local backend server and change the feature flag rule in the backend.

However, since it's a very small change in the validation file, I'd suggest you make the change, raise a PR, and our QA folks will test it on your behalf. Let me know if this works.

[harshitpandey0426]

@dhruvikn Yes I have the local backend server running. If possible let me know what changes I have to make in backend

[harshitpandey0426]

@dhruvikn I was able to test it, opened a PR please have a look

[dhruvikn]

@NilanshBansal, can we please open another issue to handle this on backend as well? Please check my thoughts here and take a call/assign anyone else as needed.

[NilanshBansal]

The BE validation is also failing to an extent. It should have thrown the error that the file size cannot exceed 1 MB, but instead, it threw the error mentioned in the issue's description. @NilanshBansal can you please check once? I'm not sure but I think it could be related to Line 97-107 of appsmith/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AssetServiceCEImpl.java

@dhruvikn the above error is not coming from backend. The backend response is as shown below.