oracle / opengrok

OpenGrok is a fast and usable source code search and cross reference engine, written in Java
http://oracle.github.io/opengrok/
Other
4.34k stars 745 forks source link

Perforce wildcards need to be protected #2746

Closed cross closed 5 years ago

cross commented 5 years ago

Hello. I am trying to set up OpenGrok 1.2.6 on an ubuntu system with multiple repositories. Some of my repositories are perforce, and among a few problems I'm facing, I'm seeing the following:

00:18:57 WARNING: Non-zero exit status 1 from command [/usr/bin/p4, files, class-map#O.ini] in directory /data/opengrok/src/myproj/dir: Invalid revision number 'O.ini'.

As you would guess, /data/opengrok/src/myproj/dir/class-map#O.ini is in fact in my sources.

Looking at https://www.perforce.com/manuals/v17.1/cmdref/Content/CmdRef/filespecs.synopsis.limitations.html I found that using HTML-escapes URI-escapes work for these characters. Testing:

p4 files ./myproj/dir/class-map%23O.ini from within my source tree succeeds and gives me the information on that class-map#O.ini file.

Please adjust your perforce repository module to understand these translations in files with reserved characters in them.

vladak commented 5 years ago

looks like the command comes from the src/main/java/org/opengrok/indexer/history/PerforceRepository.java#isInP4Depot() method. It should be sufficient to wrap that name argument in src/main/java/org/opengrok/indexer/web/Util.java#htmlize().

Pull requests are welcome.

vladak commented 5 years ago

Actually, htmlize() only works on reserved HTML characters.

vladak commented 5 years ago

The necessary conversions per the above referenced document:

char value
@ %40
# %23
* %2A
% %25
cross commented 5 years ago

Apologies, my original comment was misleading. HTML-escapes would be wrong, they need to be URI-escapes. Sorry for that. Your latest comment is correct in describing the translations needed.

Also, while changing src/main/java/org/opengrok/indexer/history/PerforceRepository.java#isInP4Depot() would avoid the issue I reported, I fear that something further down into handling perforce repository actions may also be needed. Assumedly after isInP4Depot() returns that it is (in a p4 depot), other operations would also attempt to operate on the filename and need the same change.

vladak commented 5 years ago

Definitely, had the same thought when traversing the source code, i.e. PerforceHistoryParser.java will probably need some changes as well.

tulinkry commented 5 years ago

Python tools require python3, you don’t need to worry about that, it should pass in the travis automatically.

On 22 Apr 2019, at 22:08, Chris Ross notifications@github.com wrote:

Thanks. I forked and downloaded, and made changes to PerforceHistoryParser.java and PerforceRepository.java that compile. But, unfortunately, I know enough java to work basic code, and don't know maven very well.

I was able to compile it, but ./mvnw package -DskipTests (Ubuntu 18) reports an error related to opengrok-tools. This I'm sure is an environment problem on my system unrelated to this issue, but if you could help me figure out how to get past this, I can test my changes and work towards a pull request if/when they work.

[INFO] OpenGrok Web ....................................... SUCCESS [ 3.148 s] [INFO] OpenGrok tools ..................................... FAILURE [ 0.851 s] [INFO] Distribution ....................................... SKIPPED [INFO] ------------------------------------------------------------------------ [INFO] BUILD FAILURE [INFO] ------------------------------------------------------------------------ [INFO] Total time: 15.570 s [INFO] Finished at: 2019-04-22T16:06:55-04:00 [INFO] ------------------------------------------------------------------------ [ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.6.0:exec (Generate python env) on project opengrok-tools: Command execution failed.: Process exited with an error: 1 (Exit value: 1) -> [Help 1] — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/oracle/opengrok/issues/2746#issuecomment-485535166, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVMJKBOK7PXLQL73EB4XFLPRYLNVANCNFSM4HER7BSA.

cross commented 5 years ago

@vladak pull request up, though more for review than final pull I'm sure. Feedback invited.