Skip to content
This repository has been archived by the owner on Apr 1, 2022. It is now read-only.

Fix parsing loop for req.txt files #214

Closed
wants to merge 1 commit into from

Conversation

anuccio1
Copy link
Contributor

Problem: If a requirements.txt file has a line starting with a non-letter/word character, the CLI completely hangs and appears to be stuck in a Parser loop.

To recreate this, add a requirements.txt file containing the single line: _depWithUnderscore=1.2.3, and run fossa analyze --output. The CLI will hang, and never exit.

I am very new to both Haskell and Parser's especially, so am not 100% confident that this "fix" doesn't open up any other issues, however I do know that this change fixes the original problem that I discovered

Copy link
Contributor

@skilly-lily skilly-lily left a comment

Choose a reason for hiding this comment

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

I think we're adding incorrect logic, but the changes are structurally sound.

@@ -185,7 +186,7 @@ requirementParser = specification
special <- many (oneOf ['-', '_', '.'])
lod <- letterOrDigit
pure (special ++ [lod])
identifier = label "identifier" $ (:) <$> letterOrDigit <*> (concat <$> many identifier_end)
identifier = label "identifier" $ (:) <$> (letterOrDigit <|> oneOfSpecial) <*> (concat <$> many identifier_end)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is logically correct. Python packages (specifically, modules) do not allow most special characters. I believe the only allowed characters are [a-zA-Z0-9_].

I think the correct change is this:

Suggested change
identifier = label "identifier" $ (:) <$> (letterOrDigit <|> oneOfSpecial) <*> (concat <$> many identifier_end)
identifier = label "identifier" $ (:) <$> (letterOrDigit <|> char '_') <*> (concat <$> many identifier_end)

Copy link
Member

Choose a reason for hiding this comment

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

Can one of you link to a piece of documentation for this? (Ideally, add it as a comment too.)

@skilly-lily
Copy link
Contributor

@anuccio1 can we close this PR/delete the branch in favor of #235?

@cnr
Copy link
Contributor

cnr commented May 24, 2021

Going to close this -- the issue was resolved in #235

@cnr cnr closed this May 24, 2021
@cnr cnr deleted the req-txt-underscore-parse branch May 24, 2021 20:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants