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.

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

Entry creation from DOI does not recognize URL encoding, but "get bibliographical data" does #10648

Closed
2 tasks done
DominikStiller opened this issue Nov 19, 2023 · 14 comments · Fixed by #11084
Closed
2 tasks done

Entry creation from DOI does not recognize URL encoding, but "get bibliographical data" does #10648

DominikStiller opened this issue Nov 19, 2023 · 14 comments · Fixed by #11084
Assignees
@subhramit
Labels
good first issue An issue intended for project-newcomers. Varies in difficulty. [outdated] type: bug Confirmed bugs or reports that are very likely to be bugs

Comments

@DominikStiller
Copy link

DominikStiller commented Nov 19, 2023 •
edited by koppor
Loading

Update See below. It is now about the "Entry from Id" dialog!

JabRef version

Latest development branch build (please note build date below)

Operating system

Windows

Details on version and operating system

JabRef 5.12--2023-11-16--85cd85f, Windows 10 Pro 22H2

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

  1. Find a DOI that contains URL-encoded characters such as %3C (<) or %3E( >). My example DOI: https://doi.org/10.1175/1520-0493(2002)130%3C1913:EDAWPO%3E2.0.CO;2

  2. Add a new entry from the DOI.
    Result: It fails.
    import_dialog

  3. Add an empty entry.

  4. Enter the DOI in its DOI field.

  5. Use the "get bibliographical data from DOI" function.
    Result: It works.
    lookup

Appendix

No response
@Siedlerchr Siedlerchr added the [outdated] type: bug Confirmed bugs or reports that are very likely to be bugs label Nov 20, 2023
@github-project-automation github-project-automation bot moved this to Normal priority in Prioritization Nov 20, 2023
@Siedlerchr Siedlerchr added the good first issue An issue intended for project-newcomers. Varies in difficulty. label Nov 21, 2023
@github-project-automation github-project-automation bot moved this to Free to take in Good First Issues Nov 21, 2023
@DEEPAM-KUMAR
Copy link

Hi @Siedlerchr can I work on this bug ?

@Siedlerchr Siedlerchr added the FirstTimeCodeContribution Triggers GitHub Greeter Workflow label Nov 26, 2023
@Siedlerchr Siedlerchr moved this from Free to take to Reserved in Good First Issues Nov 26, 2023
@github-actions GitHub Actions
Copy link
Contributor

As a general advice for newcomers: check out Contributing for a start. Also, guidelines for setting up a local workspace is worth having a look at.

Feel free to ask here at GitHub, if you have any issue related questions. If you have questions about how to setup your workspace use JabRef's Gitter chat. Try to open a (draft) pull-request early on, so that people can see you are working on the issue and so that they can see the direction the pull request is heading towards. This way, you will likely receive valuable feedback.

@rahul-s-bhatt
Copy link

Hey @DEEPAM-KUMAR are you still working on this?

@Siedlerchr
Copy link
Member

Seems like nobody is working on this issue

@randhirrkumar
Copy link

Hi @DominikStiller,
Actually the issue was in the URL which you are passing. You can pass ID. It will pick up the article and work properly.
I have attached 2 screenshots for your reference.
Screenshot (309)
Screenshot (310)

@DominikStiller
Copy link
Author

@randhirrkumar, I did not realize that, thank you! That's a workaround at least.

Still the behavior is inconsistent because it is generally possible to pass a URL to the new entry dialog, just not one with URL-encoded characters. So there is still a bug to be fixed here.

@koppor koppor moved this from Reserved to Free to take in Good First Issues Jan 22, 2024
@koppor
Copy link
Member

koppor commented Jan 22, 2024

Implementation steps:

  1. Add test to org.jabref.model.entry.identifier.DOITest
  2. Fix org.jabref.model.entry.identifier.DOI to urldecode URL before interpreting

@MashidAkhtar
Copy link

MashidAkhtar commented Jan 23, 2024 •
edited
Loading

Hey @koppor, can you please elaborate a few on this that you have mentioned "Fix org.jabref.model.entry.identifier.DOI to urldecode URL before interpreting"

@koppor
Copy link
Member

koppor commented Jan 24, 2024

@MashidAkhtar

I meant: At org.jabref.model.entry.identifier.DOI#DOI, right after trimmedDOI normalize the URI using the answer at https://stackoverflow.com/a/61208742/873282. Please handline nulls, too. Because the doi parameter might not be an URL

@koppor koppor moved this from Free to take to Reserved in Good First Issues Jan 28, 2024
@koppor koppor moved this from Free to take to Reserved in Candidates for University Projects Jan 28, 2024
@KonstantinosChanioglou
Copy link
Contributor

Hi. I would like to wotk on this issue. Could you assign this to me? Thanks in advance!

KonstantinosChanioglou added a commit to KonstantinosChanioglou/jabref that referenced this issue Mar 16, 2024
@KonstantinosChanioglou
Fix for Issue JabRef#10648
63b2bb1 
At org.jabref.model.entry.identifier.DOI#DOI, the method which the trimmedDoi is decoded has been modified in the case trimmedDoi being an HTTP URL.
Merged
@koppor
Copy link
Member

koppor commented Mar 19, 2024

Issue not fully fixed by #11036, missing:

"New entry from Id" is not fixed.

Import

github-merge-queue bot pushed a commit that referenced this issue Mar 19, 2024
@KonstantinosChanioglou
Allow encoded HTTP URL to create new entry (#11036)
9babea4 
* Fix for Issue #10648

At org.jabref.model.entry.identifier.DOI#DOI, the method which the trimmedDoi is decoded has been modified in the case trimmedDoi being an HTTP URL.

* PR corrections

1)java.net.URLDecoder.decode(String, Charset) is now used
2)Tests from acceptEncodedDoiUrl moved to testData
3)rewriteRun task has been executed

* PR corrections

1)Change Test/comment place
@koppor koppor removed the FirstTimeCodeContribution Triggers GitHub Greeter Workflow label Mar 19, 2024
@koppor koppor moved this from Reserved to Free to take in Candidates for University Projects Mar 19, 2024
@koppor koppor moved this from Reserved to Free to take in Good First Issues Mar 19, 2024
@subhramit
Copy link
Member

Hi, I'd like to be assigned to this.

@koppor
Copy link
Member

koppor commented Mar 22, 2024

@subhramit This is kind of detective work to understand how JabRef treats Strings coming from there.

@ThiloteE ThiloteE moved this from Free to take to Reserved in Candidates for University Projects Mar 22, 2024
@ThiloteE ThiloteE moved this from Free to take to Reserved in Good First Issues Mar 22, 2024
@subhramit
Copy link
Member

subhramit commented Mar 23, 2024 •
edited
Loading

@subhramit This is kind of detective work to understand how JabRef treats Strings coming from there.

Yes, I finally got it, it was extremely hard to debug. Many hours of breakpoint after breakpoint and turns out that the issue was because of a non-matching regex which was truncating the trailing ";2".
Will write a detailed analysis and create a PR soon.

@subhramit subhramit mentioned this issue Mar 23, 2024
Merged
6 tasks
@github-project-automation github-project-automation bot moved this from Normal priority to Done in Prioritization Mar 24, 2024
@github-project-automation github-project-automation bot moved this from Reserved to Done in Good First Issues Mar 24, 2024
@subhramit subhramit mentioned this issue Mar 31, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@subhramit subhramit

Labels
good first issue An issue intended for project-newcomers. Varies in difficulty. [outdated] type: bug Confirmed bugs or reports that are very likely to be bugs
Projects
Candidates for University Projects
Archived in project
Good First Issues
Archived in project
Prioritization
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix DOI URL parsing subhramit/jabref
9 participants
@Siedlerchr @koppor @DominikStiller @KonstantinosChanioglou @rahul-s-bhatt @DEEPAM-KUMAR @subhramit @MashidAkhtar @randhirrkumar