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

Illegal character in query: pipe character in file field #11876

Open
2 tasks done
ryan-carpenter opened this issue Oct 2, 2024 · 8 comments · May be fixed by #12156
Open
2 tasks done

Illegal character in query: pipe character in file field #11876

ryan-carpenter opened this issue Oct 2, 2024 · 8 comments · May be fixed by #12156
Assignees
Labels
📍 Assigned Assigned by assign-issue-action (or manually assigned) fetcher good first issue An issue intended for project-newcomers. Varies in difficulty.

Comments

@ryan-carpenter
Copy link

JabRef version

Other (please describe below)

Operating system

GNU / Linux

Details on version and operating system

No response

Checked with the latest development build (copy version output from About dialog)

  • I made a backup of my libraries before testing the latest development version.
  • I have tested the latest development version and the problem persists

Steps to reproduce the behaviour

I think this exception was trigger by searching full text online, which added a URL to the file field. The important part is that a pipe character in the URL raised an exception that recurred until l I killed JabRef and removed the offending URL from the .bib file.

  1. Add a URL containing a pipe (|) character to the file field (URLs from GALE ONEFILE, containing the string &docId=GALE|)

Appendix

JabRef 6.0--2024-09-25--91d0709
Linux 6.9.9-1-default amd64
Java 21.0.2
JavaFX 23+29

Log File
java.lang.IllegalArgumentException: Illegal character in query at index 58: https://go.gale.com/ps/i.do?p=HRCA&u=googlescholar&id=GALE|A653693412&v=2.1&it=r&sid=googleScholar&asid=68b1dec7
	at java.base/java.net.URI.create(Unknown Source)
	at [email protected]/org.jabref.logic.importer.util.FileFieldParser.convert(Unknown Source)
	at [email protected]/org.jabref.logic.importer.util.FileFieldParser.parse(Unknown Source)
	at [email protected]/org.jabref.logic.importer.util.FileFieldParser.parse(Unknown Source)
	at java.base/java.util.Optional.map(Unknown Source)
	at [email protected]/com.tobiasdiez.easybind.optional.PreboundOptionalBinding$1.computeValue(Unknown Source)
	at [email protected]/com.tobiasdiez.easybind.optional.PreboundOptionalBinding$1.computeValue(Unknown Source)
	at javafx.base@23/javafx.beans.binding.ObjectBinding.get(Unknown Source)
	at javafx.base@23/javafx.beans.binding.ObjectExpression.getValue(Unknown Source)
	at [email protected]/com.tobiasdiez.easybind.optional.PreboundOptionalBinding$3.computeValue(Unknown Source)
	at javafx.base@23/javafx.beans.binding.ObjectBinding.get(Unknown Source)
	at javafx.base@23/javafx.beans.binding.ObjectExpression.getValue(Unknown Source)
	at javafx.controls@23/javafx.scene.control.TableCell.updateItem(Unknown Source)
	at javafx.controls@23/javafx.scene.control.TableCell.indexChanged(Unknown Source)
	at javafx.controls@23/javafx.scene.control.IndexedCell.updateIndex(Unknown Source)
	at javafx.controls@23/javafx.scene.control.skin.TableRowSkinBase.updateCells(Unknown Source)
	at javafx.controls@23/javafx.scene.control.skin.TableRowSkinBase.checkState(Unknown Source)
	at javafx.controls@23/javafx.scene.control.skin.TableRowSkinBase.computePrefHeight(Unknown Source)
	at javafx.controls@23/javafx.scene.control.Control.computePrefHeight(Unknown Source)
	at javafx.graphics@23/javafx.scene.Parent.prefHeight(Unknown Source)
	at javafx.graphics@23/javafx.scene.layout.Region.prefHeight(Unknown Source)
	at javafx.controls@23/javafx.scene.control.skin.VirtualFlow.resizeCell(Unknown Source)
	at javafx.controls@23/javafx.scene.control.skin.VirtualFlow.getCell(Unknown Source)
	at javafx.controls@23/javafx.scene.control.skin.VirtualFlow.getOrCreateCellSize(Unknown Source)
	at javafx.controls@23/javafx.scene.control.skin.VirtualFlow.getOrCreateCellSize(Unknown Source)
	at javafx.controls@23/javafx.scene.control.skin.VirtualFlow.recalculateAndImproveEstimatedSize(Unknown Source)
	at javafx.controls@23/javafx.scene.control.skin.VirtualFlow.recalculateAndImproveEstimatedSize(Unknown Source)
	at javafx.controls@23/javafx.scene.control.skin.VirtualFlow.recalculateEstimatedSize(Unknown Source)
	at javafx.controls@23/javafx.scene.control.skin.VirtualFlow.layoutChildren(Unknown Source)
	at javafx.graphics@23/javafx.scene.Parent.layout(Unknown Source)
	at javafx.graphics@23/javafx.scene.Parent.layout(Unknown Source)
	at javafx.graphics@23/javafx.scene.Parent.layout(Unknown Source)
	at javafx.graphics@23/javafx.scene.Parent.layout(Unknown Source)
	at javafx.graphics@23/javafx.scene.Parent.layout(Unknown Source)
	at javafx.graphics@23/javafx.scene.Parent.layout(Unknown Source)
	at javafx.graphics@23/javafx.scene.Parent.layout(Unknown Source)
	at javafx.graphics@23/javafx.scene.Parent.layout(Unknown Source)
	at javafx.graphics@23/javafx.scene.Parent.layout(Unknown Source)
	at javafx.graphics@23/javafx.scene.Parent.layout(Unknown Source)
	at javafx.graphics@23/javafx.scene.Scene.doLayoutPass(Unknown Source)
	at javafx.graphics@23/javafx.scene.Scene$ScenePulseListener.pulse(Unknown Source)
	at javafx.graphics@23/com.sun.javafx.tk.Toolkit.lambda$runPulse$2(Unknown Source)
	at java.base/java.security.AccessController.doPrivileged(Unknown Source)
	at javafx.graphics@23/com.sun.javafx.tk.Toolkit.runPulse(Unknown Source)
	at javafx.graphics@23/com.sun.javafx.tk.Toolkit.firePulse(Unknown Source)
	at javafx.graphics@23/com.sun.javafx.tk.quantum.QuantumToolkit.pulse(Unknown Source)
	at javafx.graphics@23/com.sun.javafx.tk.quantum.QuantumToolkit.pulse(Unknown Source)
	at javafx.graphics@23/com.sun.javafx.tk.quantum.QuantumToolkit.pulseFromQueue(Unknown Source)
	at javafx.graphics@23/com.sun.javafx.tk.quantum.QuantumToolkit.lambda$runToolkit$11(Unknown Source)
	at javafx.graphics@23/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(Unknown Source)
	at javafx.graphics@23/com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
	at javafx.graphics@23/com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$10(Unknown Source)
	at java.base/java.lang.Thread.run(Unknown Source)
Caused by: java.net.URISyntaxException: Illegal character in query at index 58: https://go.gale.com/ps/i.do?p=HRCA&u=googlescholar&id=GALE|A653693412&v=2.1&it=r&sid=googleScholar&asid=68b1dec7
	at java.base/java.net.URI$Parser.fail(Unknown Source)
	at java.base/java.net.URI$Parser.checkChars(Unknown Source)
	at java.base/java.net.URI$Parser.parseHierarchical(Unknown Source)
	at java.base/java.net.URI$Parser.parse(Unknown Source)
	at java.base/java.net.URI.<init>(Unknown Source)
	... 53 more
@HoussemNasri
Copy link
Member

HoussemNasri commented Oct 5, 2024

Seems like a problem with the underlying URI implementation in the JDK, they implement RFC2396 + RFC2732 while the usage of pipe characters is only supported in RFC3986.

Source: gatling/gatling#1981 (comment).

Solution: Encode pipe characters as %7C before passing them into the URI.

raised an exception that recurred until l I killed JabRef and removed the offending URL from the .bib file.

This is bad. This type of exceptions should never make the program unusable.

@jiaxin0103
Copy link

hi @ryan-carpenter , Is this issue still available for contribution? I have a student assignment that requires contributing to an open-source project, and I’m interested in contributing to this one.Could you assign this issue to me?

@ThiloteE ThiloteE assigned jiaxin0103 and unassigned jiaxin0103 Oct 9, 2024
@ThiloteE
Copy link
Member

ThiloteE commented Oct 9, 2024

@jiaxin0103 Please have a look at https://github.com/orgs/JabRef/projects/5 and choose a "free to take" issue, as it depends on the JDK. It would be needed to open a PR over there, if a solution is found.

Edit: Sorry, this is not search related. My bad. Not sure, why I completely missed HoussemNasri's comment.

@ThiloteE ThiloteE added fetcher status: depends-on-external A bug or issue that depends on an update of an external library labels Oct 9, 2024
@koppor
Copy link
Member

koppor commented Oct 16, 2024

I think, this can be done inside JabRef, too. As stated at #11876 (comment), one needs to recode the pipe sign into %7C.

Replace all calls to URI.create with a new method.

  1. Create a new method create in org.jabref.gui.fieldeditors.URLUtil. That method returns an URL It makes use of createUri and then calls toUrl()
  2. Create a new method createUri in org.jabref.gui.fieldeditors.URLUtil. That method returns an URI.
  3. Replace all calls of URI.create(...).toURL() by URLUtil.create(...).
  4. Replace all calls of URI.create(...) by URLUtil.createUri(...).

The method in step 2 does that escaping.

Test cases need to be added to org.jabref.logic.net.URLUtilTest.

@koppor koppor removed the status: depends-on-external A bug or issue that depends on an update of an external library label Oct 16, 2024
@koppor
Copy link
Member

koppor commented Oct 16, 2024

@jiaxin0103 Please reply with /assign-me if you want to take this issue.

@koppor koppor added the good first issue An issue intended for project-newcomers. Varies in difficulty. label Oct 16, 2024
@github-project-automation github-project-automation bot moved this to Free to take in Good First Issues Oct 16, 2024
@github-actions github-actions bot added the 📍 Assigned Assigned by assign-issue-action (or manually assigned) label Oct 16, 2024
@koppor koppor removed the 📍 Assigned Assigned by assign-issue-action (or manually assigned) label Oct 16, 2024
@koppor koppor removed their assignment Oct 16, 2024
@hitalo-siriano
Copy link

Hello @koppor , I would like to work on this problem.

@JabRef JabRef deleted a comment from github-actions bot Oct 20, 2024
@koppor
Copy link
Member

koppor commented Oct 20, 2024

/assign @hitalo-siriano

Copy link
Contributor

👋 Hey @hitalo-siriano, thank you for your interest in this issue! 🎉

We're excited to have you on board. Start by exploring our Contributing guidelines, and don't forget to check out our workspace setup guidelines to get started smoothly.

In case you encounter failing tests during development, please check our developer FAQs!

Having any questions or issues? Feel free to ask here on GitHub. Need help setting up your local workspace? Join the conversation on JabRef's Gitter chat. And don't hesitate to open a (draft) pull request early on to show the direction it is heading towards. This way, you will receive valuable feedback.

Happy coding! 🚀

⏳ Please note, you will be automatically unassigned if the issue isn't closed within 30 days (by 19 November 2024). A maintainer can also add the "📌 Pinned"" label to prevent automatic unassignment.

@github-actions github-actions bot added the 📍 Assigned Assigned by assign-issue-action (or manually assigned) label Oct 20, 2024
@koppor koppor moved this from Free to take to Assigned in Good First Issues Oct 20, 2024
hitalo-siriano added a commit to hitalo-siriano/jabref that referenced this issue Oct 22, 2024
Modified the handling of URLs in the file field to replace the pipe character (|) with its encoded representation (%7C) to avoid exceptions
in JabRef.

Closes JabRef#11876
hitalo-siriano added a commit to hitalo-siriano/jabref that referenced this issue Nov 5, 2024
…loses JabRef#11876.

- Introduced URLUtil.createUri() and URLUtil.create() to handle URL sanitization.
- Replaced direct calls to URI.create() and URI.create().toURL() with the new utility methods.
- URLs containing the pipe character ('|') are now properly encoded as '%7C' to prevent parsing errors.
- Added test cases to URLUtilTest to verify correct sanitization and URL creation.
hitalo-siriano added a commit to hitalo-siriano/jabref that referenced this issue Nov 16, 2024
…loses JabRef#11876.

- Introduced URLUtil.createUri() and URLUtil.create() to handle URL sanitization.
- Replaced direct calls to URI.create() and URI.create().toURL() with the new utility methods.
- URLs containing the pipe character ('|') are now properly encoded as '%7C' to prevent parsing errors.
- Added test cases to URLUtilTest to verify correct sanitization and URL creation.
- Added @archtest to ensure that the URI.create() method is not directly called in the codebase.
hitalo-siriano added a commit to hitalo-siriano/jabref that referenced this issue Nov 17, 2024
…loses JabRef#11876.

- Introduced URLUtil.createUri() and URLUtil.create() to handle URL sanitization.
- Replaced direct calls to URI.create() and URI.create().toURL() with the new utility methods.
- URLs containing the pipe character ('|') are now properly encoded as '%7C' to prevent parsing errors.
- Added test cases to URLUtilTest to verify correct sanitization and URL creation.
- Added @archtest to ensure that the URI.create() method is not directly called in the codebase.
hitalo-siriano added a commit to hitalo-siriano/jabref that referenced this issue Nov 17, 2024
…loses JabRef#11876.

- Introduced URLUtil.createUri() and URLUtil.create() to handle URL sanitization.
- Replaced direct calls to URI.create() and URI.create().toURL() with the new utility methods.
- URLs containing the pipe character ('|') are now properly encoded as '%7C' to prevent parsing errors.
- Added test cases to URLUtilTest to verify correct sanitization and URL creation.
- Added @archtest to ensure that the URI.create() method is not directly called in the codebase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📍 Assigned Assigned by assign-issue-action (or manually assigned) fetcher good first issue An issue intended for project-newcomers. Varies in difficulty.
Projects
Status: In Progress
6 participants