8000 Opengrok sends HTTP requests without fragment references · Issue #4791 · oracle/opengrok · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Opengrok sends HTTP requests without fragment references #4791

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.

Sign up for GitHub

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

Jump to bottom
Open
alighaddar10 opened this issue Jun 3, 2025 · 5 comments
Open

Opengrok sends HTTP requests without fragment references #4791

alighaddar10 opened this issue Jun 3, 2025 · 5 comments
Labels
authorization enhancement webapp web application

Comments

@alighaddar10
Copy link

Hello team,

Describe the bug
After implementing a single sign-on solution for opengrok on the level of the HAProxy inspired by the issue previously opened on the topic, we noticed a bug in a use-case of opengrok. Whenever a user is opening a link that contains a fragment, for example https://opengrok.example.com/prod/src/build.cpp#getBuild that points to the getBuild() method line, the redirect to authenticate and generate an access token leads the user to https://opengrok.example.com/prod/src/build.cpp thus losing the fragment that points to the specific section. Further investigation lead us to find that the request reaching the proxy already has the fragment reference stripped out, blocking us from preserving it on the proxy level.

To Reproduce
Using an SSO secured opengrok instance, try to open a link containing a fragment reference in the URL without having previously authenticated and having a valid access token. It should lead you to the file without the fragment reference still being present in the URL. This behavior is consistent across browsers and even on cURL with follow redirects enabled.

We are using a custom implementation of Single Sign-On authentication and authorization inspired by the following discussion (#4723) with adaptations taylored for our environment and stack.

Expected behavior
The URL should retain the fragment reference and send it in the request.

Additional context
A suggested solution is to have the reference be a query parameter instead of a fragment reference to be able to receive and parse it to retain the reference when sharing a link to a file. Another solution is to have the fragment reference forcefully retained in the http request if applicable depending on your specific implementation of the client and server side connections.

Please let us know if you need further clarification or if you have another solution in mind to work around this issue. It is blocking for us as it creates an extra barrier for users access opengrok links from documentations which is one of our most common usecases of the service.

Thank you for all your efforts,
Sincerely,
Ali
@vladak
Copy link
Member
vladak commented Jun 3, 2025 •
edited
Loading

Indeed, it seems that the current behavior of various HTTP clients, including browsers, is not to send the fragment identifier in the HTTP request. This means there is no way for it to propagate it through all the authentication redirects, even though the Location HTTP header allows for it in redirects.

The way to pass the fragment identifier is, as you suggest, via a request parameter. This is actually already done for the annotate view to preserve the line numbers using the h parameter:

opengrok/opengrok-web/src/main/webapp/js/utils-0.0.47.js

Lines 1677 to 1682 in e34493b

if (!window.location.hash) {
const h = getParameter("h");
if (h && h !== "") {
window.location.hash = h;
}
}

opengrok/opengrok-web/src/main/webapp/js/utils-0.0.47.js

Lines 2028 to 2036 in e34493b

if (window.location.hash) {
// If a line is highlighted when "annotate" is clicked, we want to
// preserve the highlighting, but we don't want the page to scroll
// to the highlighted line. So put the line number in a URL parameter
// instead of in the hash.
link += "&h=";
link += window.location.hash.substring(1, window.location.hash.length);
}
window.location = link;

Given that the piece which extracts the fragment identifier from the h parameter is in domReadyMast (i.e. the consumer), then making this work is just a matter of setting the parameter in window.location, I think.

Even when passing the fragment identifer in the request, the hash links should be preserved to avoid breaking links embedded in bug tracking systems, etc.

Lastly, the hash can consist of two numbers (designating a line range rather than single line) so this needs to be taken into account when encoding it in the URI.

@alighaddar10
Copy link
Author

Can you provide us with more details please as to the adaptation needed to make this work? I tested the 'h' parameter and indeed it translates into an anchor. However, the issue is that all the documentations and references use links with anchors as it stands, and it would be very tedious to go over them and change them. Also, kindly note that using the navigate functionality as it stands doesn't generate the parameter but only the anchor, making it even more tedious for users to generate links. Is there a way to enforce the parameter or maybe always translate it? Kindly let us know if there is a solution we can implement that can circumvent these obstacles.

vladak added a commit to vladak/OpenGrok that referenced this issue Jun 4, 2025
@vladak
add the 'h' request parameter for line selections
6e27984 
approaches oracle#4791
@vladak
Copy link
Member
vladak commented Jun 4, 2025 •
edited
Loading

There is no way how to fix this problem for pre-existing links as far as I know due to the reason described above.

For new links, the changes need to happen in two places:

  1. in the utils JS code the h parameter needs to be added whenever a line (range) is selected
  2. as you noted, the links in the xref needed to be changed as well. This can be done either in the code which produces the xrefs (which would require reindexin 8000 g from scratch) or alternatively in the JS code to change all these links on the fly (which would probably slow down page rendering a bit).
    - also, this is not only needed for the Navigate pane links but also for intra-document links (e.g. for locally defined variables and such)

Some work in progress can be seen in https://github.com/vladak/OpenGrok/tree/webapp_frag_ids_via_h_param, notably vladak@6e27984 which appears to do the job for the first item above. I have no capacity to finish this at the moment.

@alighaddar10
Copy link
Author

Thank you so much for your efforts and reactivity! We will look into how we can assist with this fix. Please keep us posted when you make progress on this issue on your time of course.

Regarding the pre-existing links, I figured as such and we're brainstorming a way to migrate links to the new format.

Many thanks again.

@vladak
Copy link
Member
vladak commented Jun 5, 2025 •
edited
Loading

For the approach with links changed during indexing, I believe most of the links are generated via the methods defined in https://github.com/oracle/opengrok/blob/master/opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/JFlexXrefUtils.java, for example

opengrok/opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/JFlexXrefUtils.java

Lines 311 to 315 in e34493b

// Generate a direct link to the symbol definition.
out.append("<a class=\"");
out.append(styleClass);
out.append(" intelliWindow-symbol\" href=\"#");
Util.uriEncode(symbol, out);

Anyhow, if changed there, this would probably lead to some churn w.r.t. pre-existing xref analyzer tests where xref output is compared to known golden output for given input file.

Further, I am not sure what will happen if these purely anchor links will get converted to full, albeit relative, links. Specifically whether browsers will be able to recognize these links point to the currently displayed page and avoid extra HTTP request.

Also, the line number links themselves (those in the leftmost column) have to be changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
authorization enhancement webapp web application
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@vladak @alighaddar10
0