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

[GS] use the request's basePath when processing server-side result urls #76747

Merged

Conversation

pgayvallet
Copy link
Contributor

@pgayvallet pgayvallet commented Sep 4, 2020

Summary

Related to #72331

Fix a bug causing the server-side GS results to use the server's basePath instead of the inbound request's when processing result urls.

Checklist

@pgayvallet pgayvallet added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Sep 4, 2020
@pgayvallet pgayvallet marked this pull request as ready for review September 4, 2020 11:31
@pgayvallet pgayvallet requested a review from a team as a code owner September 4, 2020 11:31
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

distributable file count

id value diff baseline
default 45410 +1 45409

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

): BasePathAccessor => {
const requestBasePath = basePath.get(request);
return {
prepend: (path) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to reuse basePath.prepend here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK there is no way to only retrieve the request-specific (requestScopePath) part of the basePath via the service, as get returns the 'whole' base path:

/**
* returns `basePath` value, specific for an incoming request.
*/
public get = (request: KibanaRequest | LegacyRequest) => {
const requestScopePath = this.basePathCache.get(ensureRawRequest(request)) || '';
return `${this.serverBasePath}${requestScopePath}`;
};

and prepend only allow to preprend using the server's base path, not a request-scoped basePath:

public prepend = (path: string): string => {
if (this.serverBasePath === '') return path;
return modifyUrl(path, (parts) => {
if (!parts.hostname && parts.pathname && parts.pathname.startsWith('/')) {
parts.pathname = `${this.serverBasePath}${parts.pathname}`;
}
});
};

But maybe adding a new core API is better. I was surprised to be the first to have such need on the server-side TBH.

I can add something like prependRequestPath(path: string, request: KibanaRequest) to the server-side BasePath?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants