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

URLExt.join behaviour for protocol-relative URLs changed in 3.0.10 #13324

Open
vipcxj opened this issue Oct 27, 2022 · 6 comments
Open

URLExt.join behaviour for protocol-relative URLs changed in 3.0.10 #13324

vipcxj opened this issue Oct 27, 2022 · 6 comments
Labels
api-change A change that should be accompanied by a major version increase bug pkg:coreutils

Comments

@vipcxj
Copy link

vipcxj commented Oct 27, 2022

Description

Kite extension doesn't work for Jupyter, Someone has report this bug. But the jupyterlab developer think it is caused by kite and closed the issue. After some research, I find out it may be the bug of jupyterlab.
Here is the link. I modify the file according @Jordan20190913 . and works. The file "jlab_core.d4ca3e2ff3a1a5677f23.js" should belong to jupyterlab.

Reproduce

Just install jupyter-kite with latest jupyterlab.

Expected behavior

kite should works

Context

  • Operating System and version: Linux Ubuntu 20.04
  • Browser and version: Chrome 92
  • JupyterLab version: 3.5.0
Troubleshoot Output
Paste the output from running `jupyter troubleshoot` from the command line here.
You may want to sanitize the paths in the output.
Command Line Output
Paste the output from your command line running `jupyter lab` here, use `--debug` if possible.
Browser Output
Paste the output from your browser Javascript console here, if applicable.
@vipcxj vipcxj added the bug label Oct 27, 2022
@jupyterlab-probot jupyterlab-probot bot added the status:Needs Triage Applied to new issues that need triage label Oct 27, 2022
@welcome
Copy link

welcome bot commented Oct 27, 2022

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@JasonWeill
Copy link
Contributor

The Kite extension is no longer offered for download by the Kite Corporation on its web site, although their open source repositories remain available. According to kiteco/issue-tracker#767 , the Kite extension is compatible with older versions of JupyterLab (3.2.x) but not with newer, more actively maintained versions. The most recent commit to the JupyterLab Kite repo was on October 25, 2021, just over a year ago.

I'm going to close this issue out because, unfortunately, the Kite extension doesn't seem to be actively maintained.

@vipcxj
Copy link
Author

vipcxj commented Oct 28, 2022

@jweill-aws Even if the Kite extension doesn't seem to be actively maintained, with this workaround, it is still compatible with the latest jupyterlab. And it seems that the workaround is not for kite, but for jupyterlab. It seems the bug of jupyterlab, not of kite.

@JasonWeill
Copy link
Contributor

@vipcxj Thanks for clarifying! I've reopened this issue. Please feel free to submit a pull request to improve JupyterLab's compatibility with this extension.

@JasonWeill JasonWeill reopened this Oct 28, 2022
@JasonWeill JasonWeill removed the status:Needs Triage Applied to new issues that need triage label Oct 28, 2022
@krassowski
Copy link
Member

The reasoning here is incorrect: the fact that a change in JupyterLab can workaround an issue you experience in a non-maintained third-party extension does not imply that the fault lies in JupyterLab.

Context: The upstream dependency used by JupyterLab to parse URLs (url-parse) has changed how it handles some edge cases (URLs for file schema). This was due to a rewrite of some url-parse logic which was necessary for security reasons hence the new version was included in JupyterLab. Unfortunately that rewrite fixing security vulnerabilities also introduced a few issues which we picked up:

While most of the issues have been fixed, some of the behaviour changes were retained and described as intentional/wont-fix by the url-parse team as they follow the specification/reference implementation: unshiftio/url-parse#219 (comment).

This necessitates a change in the extension to update the expectation, a thing that I found no good solution to. Instead I implemented a workaround in jupyterlab-lsp which "just works" (for now): jupyter-lsp/jupyterlab-lsp#599. Since the Kite extension is largely based on jupyterlab-lsp (reusing a lot of code), it would need a similar workaround. Someone tried to do so in kiteco/jupyterlab-kite#73.

While the workaround that you link to, which changes implementation of URLExt.join, is indeed working, it could not be backported to JupyterLab 3.x because it would again change the behaviour for everyone, introducing downstream issues (for example for jupyterlab-lsp). We could introduce a behaviour change in 4.x but only if it was a very deliberate change with extensive test coverage ensuring no regressions. On the other hand, since jupyter-lsp machinery is included in core since 4.0 all of this will be a non-issue for these kind of extensions and a change annoying maintained third-party extension authors will not be needed.

However, if you do feel like exploring this in more details, please feel welcome to create a PR with such modification and extensive test coverage, targeting next major release (currently 4.0/master branch). The relevant code section is:

export function join(...parts: string[]): string {
let u = urlparse(parts[0], {});
// Schema-less URL can be only parsed as relative to a base URL
// see https://github.com/unshiftio/url-parse/issues/219#issuecomment-1002219326
const isSchemaLess = u.protocol === '' && u.slashes;
if (isSchemaLess) {
u = urlparse(parts[0], 'https:' + parts[0]);
}
const prefix = `${isSchemaLess ? '' : u.protocol}${u.slashes ? '//' : ''}${
u.auth
}${u.auth ? '@' : ''}${u.host}`;
// If there was a prefix, then the first path must start at the root.
const path = posix.join(
`${!!prefix && u.pathname[0] !== '/' ? '/' : ''}${u.pathname}`,
...parts.slice(1)
);
return `${prefix}${path === '.' ? '' : path}`;
}

@krassowski krassowski changed the title Kite extension doesn't work for Jupyter URLExt.join behaviour for protocol-relative URLs changed in 3.0.10 Oct 28, 2022
@krassowski
Copy link
Member

I updated the title to reflect the core issue. I just want to stress that changing the behaviour back to how it was in 3.0.10 might go against the specification (I am not convinced here), so any PR needs to be presented with a good research and strong rationale.

@krassowski krassowski added pkg:coreutils api-change A change that should be accompanied by a major version increase labels Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change A change that should be accompanied by a major version increase bug pkg:coreutils
Projects
None yet
Development

No branches or pull requests

3 participants