Bug 62878 - includecontroller.prefix fails when path separator not added
Summary: includecontroller.prefix fails when path separator not added
Status: NEW
Alias: None
Product: JMeter
Classification: Unclassified
Component: Main (show other bugs)
Version: 4.0
Hardware: PC All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-02 00:49 UTC by Andrew Burton
Modified: 2018-11-04 22:57 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Burton 2018-11-02 00:49:41 UTC
Specifying the includecontroller.prefix fails if the property or the specified include filename don't end/start with a path separator.

For example:
includecontroller.prefix=/path/to/fragments
Include Controller filename = myfragment.jmx
results in the log message:
\"/path/to/fragmentsmyfragment.jmx\" not found for Include Controller \"Include Controller - My Fragment\"

This is due to the string append used in IncludeController.java:

String fileName=prefix+includePath;
File file = new File(fileName);

Suggest the code be amended to:

File file = Paths.get(prefix, includePath).toFile();

This way, the user input is handled gracefully. This assumes that the prefix is a directory.
Comment 1 Philippe Mouawad 2018-11-03 22:31:43 UTC
(In reply to Andrew Burton from comment #0)
> Specifying the includecontroller.prefix fails if the property or the
> specified include filename don't end/start with a path separator.
> 
> For example:
> includecontroller.prefix=/path/to/fragments
> Include Controller filename = myfragment.jmx
> results in the log message:
> \"/path/to/fragmentsmyfragment.jmx\" not found for Include Controller
> \"Include Controller - My Fragment\"
> 
> This is due to the string append used in IncludeController.java:
> 
> String fileName=prefix+includePath;
> File file = new File(fileName);
> 
> Suggest the code be amended to:
> 
> File file = Paths.get(prefix, includePath).toFile();
> 
> This way, the user input is handled gracefully. This assumes that the prefix
> is a directory.

Thanks for report and analysis.

Still, I see nowhere in documentation that "includecontroller.prefix" MUST be a folder. 
So if we implement your proposal (which I find a good proposal), we could break existing plans.
Did you see in current documentation that would confirm includecontroller.prefix should be a folder ? If yes, we could apply your proposal without risk.

Thanks
Comment 2 Andrew Burton 2018-11-04 22:57:19 UTC
Yes, I did think there might be a set of users who use the prefix in this way.

Looking at the other properties that use prefix or suffix, these seem to be tied to the file's basename, not as part of a full pathname. So to have a prefix implies you are enforcing a naming convention for how the included JMX should be named, not specifying where it is located. That would be more intuitive if it was called, say, includecontroller.path.

I definitely think being able to specify a path for included JMX files is warranted, especially since the field is not programmable via the __P function. And I think it is relatively *unintuitive* to have a single field  specifying a path and optionally a prefix. The edge cases for trying to determine whether a user meant a path or a path + prefix would be a pain to handle.

Would this bug report be better as a request for a new property includecontroller.path. That way we can easily handle a user including/not including a trailing path. The prefix would be string appended to the filename and the full path built from that using the Java nio functions.

A message on the release notes to state the change.

Current users:
- users who put JMeter in the "expected" locations, and use the prefix as a way of shortening the name in the filename field - these users would be unaffected.
- users who specify a path as the property - these users would be unaffected (presumably they have already been caught by the original issue on trailing file separators I reported)
- users who specify a path + first part of the filename - these users would be affected. Without usage patterns it's difficult to know, but I imagine most users of this property would fall into the first 2 categories.