Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perforce wildcards need to be protected #2746

Closed
cross opened this issue Apr 9, 2019 · 7 comments
Closed

Perforce wildcards need to be protected #2746

cross opened this issue Apr 9, 2019 · 7 comments

Comments

@cross
Copy link
Contributor

cross commented Apr 9, 2019

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 vladak added the bug label Apr 9, 2019
@vladak
Copy link
Member

vladak commented Apr 9, 2019

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
Copy link
Member

vladak commented Apr 9, 2019

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

@vladak
Copy link
Member

vladak commented Apr 9, 2019

The necessary conversions per the above referenced document:

char value
@ %40
# %23
* %2A
% %25

@cross
Copy link
Contributor Author

cross commented Apr 9, 2019

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
Copy link
Member

vladak commented Apr 11, 2019

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

@tulinkry
Copy link
Contributor

tulinkry commented Apr 23, 2019 via email

@cross
Copy link
Contributor Author

cross commented Apr 30, 2019

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

cross pushed a commit to cross/opengrok that referenced this issue May 8, 2019
@vladak vladak closed this as completed in 455d807 May 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants