Opengrok sends HTTP requests without fragment references

[alighaddar10]

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]

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]

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]

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 reindexing 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]

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.