Hi Yang,

I also got that error when trying to upload the image using the "Add
Additional Images" screen in Catalog -> Products -> (Select a product) ->
Content.
When using the "Upload Image" in the bottom part of "Override Simple
Fields" screen. Not sure why we get this.

 Hi Jacques,

Option 1
Is not going to work because checkMaxLinesLength throw an
exception(java.nio.charset.MalformedInputException: Input length = 1) at
line:
-  List<String> lines = FileUtils.readLines(file, Charset.defaultCharset());

So it never gets to check the line length.

Option 2
This should work. I'll attach the fix to the issue you mentioned.
Unfortunately ATM I don't have time to
further investigate this issue, but I will leave a comment to the issue
with the findings so far.

Also, quite important. When looking at the code, I could not create a unit
test for this case, because
SecuredUpload uses private static methods where it interacts with files,
thus to create an isolated
unit test for this case there are only 2 options:

1. Upload a file image in the test resources folder, that will be used in
this test.
Not really optimal, since the codebase will grow really large really fast,
if we put a lot of test resources inside.

2. Mock methods that we don't care about in this test case.
Since the methods are static, and private, we cannot mock them using
Mockito(already added as a test dependency).
The only option here to mock them would be to add PowerMockito as a test
dependency and use that.

I believe going forward we should either consider adding PowerMockito as a
test tool, or consider writing more
testable code, as I think having the ability to write easy and fast unit
tests are a must in order to maintain code stability,
and prevent introducing bugs with changes.

On Sun, Sep 8, 2024 at 9:00 PM Jacques Le Roux <jacques.le.r...@les7arts.com>
wrote:

> BTW, there is already a Jira for that:
> https://issues.apache.org/jira/browse/OFBIZ-12639
>
> Any help is welcome :)
>
> Le 08/09/2024 à 17:03, Jacques Le Roux a écrit :
> > Hi Groza,
> >
> > After facing several webshell uploads I made SecuredUpload.java as
> secure as possible OOTB.
> >
> > I see 2 options here:
> >
> >  * Increase maxLineLength in security.properties (could be unsecure, but
> not that bad)
> >  * Improve SecuredUpload by having a special treatment for Images at
> line 209
> >
> > HTH
> >
> > Jacques
> >
> > Le 07/09/2024 à 12:52, Groza Danut a écrit :
> >> Hi,
> >>
> >> Have you tried to add an image to a product? I get an error message
> saying
> >> type unsupported for security reasons, even if the file type is .jpeg.
> >>
> >> When debugging I found that ProductServices.addAdditionalViewForProduct
> is
> >> called.
> >> At line 1083: org.apache.ofbiz.security.SecuredUpload.isValidFile is
> called
> >>
> >> Inside SecuredUpload line 254: checkMaxLinesLength throws an error,
> since
> >> this is a jpeg file.
> >>
> >> As far as I see it, isValidFile should not checkMaxLinesLength if
> >> the fileType is IMAGE.
> >>
>


-- 
Groza Dănuț

Reply via email to