Bug 56877 - CSVDataSet does not trim spaces in <filename>.csv
Summary: CSVDataSet does not trim spaces in <filename>.csv
Status: NEW
Alias: None
Product: JMeter
Classification: Unclassified
Component: Main (show other bugs)
Version: unspecified
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2014-08-21 08:43 UTC by Dzmitry Kashlach
Modified: 2016-04-14 12:09 UTC (History)
3 users (show)



Attachments
csv_dataset.patch (616 bytes, patch)
2014-08-21 08:43 UTC, Dzmitry Kashlach
Details | Diff
TestBeanHelper&GenericTestBeanCustomizer (1.58 KB, patch)
2014-08-25 09:26 UTC, Dzmitry Kashlach
Details | Diff
CSVDataSet&TCLogParser (1.33 KB, patch)
2014-08-25 14:04 UTC, Dzmitry Kashlach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dzmitry Kashlach 2014-08-21 08:43:33 UTC
Created attachment 31936 [details]
csv_dataset.patch

CSVDataSet does not trim spaces in <filename>.csv
E.g. if user entered in text-box "<filename>.csv ", then filename will be set as 
"<filename>.csv " but not "<filename>.csv".
I suggest simple patch for it.
Comment 1 Andrey Pokhilko 2014-08-21 09:34:06 UTC
Maybe this is better be fixed in org.apache.jmeter.services.FileServer class to have the effect everywhere.
Comment 2 Dzmitry Kashlach 2014-08-25 09:26:18 UTC
Created attachment 31939 [details]
TestBeanHelper&GenericTestBeanCustomizer

I've made some more research and tried to fixed this issue for all String properties wherever they occur.
Comment 3 Philippe Mouawad 2014-08-25 10:46:52 UTC
I think the fix of last patch is too coarse and impacting as it hits much more than simple path in Beans.
Comment 4 Dzmitry Kashlach 2014-08-25 14:03:02 UTC
Third variant is to fix CSVDataSet and TCLogParser, because they both reserve files using FilerServer. In this case patch may look like latest one in attachement.
Comment 5 Dzmitry Kashlach 2014-08-25 14:04:08 UTC
Created attachment 31940 [details]
CSVDataSet&TCLogParser
Comment 6 Philippe Mouawad 2014-08-30 12:46:16 UTC
Hello,
Thanks for contribution.

To be clean, I think we should look at all elements that use FileNames and add trimming (not only to 2 elements).

Also, for cleanness, I suggest setter are not modified, only the code that uses fileName should trim it.
Comment 7 Dzmitry Kashlach 2014-08-30 15:44:19 UTC
(In reply to Philippe Mouawad from comment #6)
> To be clean, I think we should look at all elements that use FileNames and
> add trimming (not only to 2 elements).
 Agree. But here is a little dilemma for me: either to fix two elements(which use FileServer, CSVDataSet&TCLogParser), or make deep fix, which you've already rejected(trimm all StringProperties in TestBeanHelper&GenericTestBeanCustomizer).
Do you know any intermediate variant?

>Also, for cleanness, I suggest setter are not modified, only the code that uses >fileName should trim it.
 I think, it could be better to trim filename higher than fileserver level, because filename is used also as alias. In this case we'll do far less changes in code.
Comment 8 Philippe Mouawad 2014-08-30 15:51:51 UTC
Hi,
I rejected the trim on  all StringProperties in TestBeanHelper&GenericTestBeanCustomizer  as it would have affected also non filenames which may have side effects for existing plans.

The best thing IMHO is to review all components that have filenames and trim spaces for them.

It is more work but I think it is more coherent.
Regards
Comment 9 Antonio Gomes Rodrigues 2016-04-14 11:58:26 UTC
Hi,


I would like to know if you think it's need to be fixed?

Because have spaces at the begin or/and at the end of a file is authorized (tested in Windows + Linux), I don't know what it needed to do

Do you think the issue must be closed?

If not, I will split it in separate issue and fix them one by one

Antonio
Comment 10 Philippe Mouawad 2016-04-14 12:09:33 UTC
Hi Antonio,  
if you can take into account my previous comment, then yes a PR is welcome.

Regards