Doctrine Common
  1. Doctrine Common
  2. DCOM-118

[GH-195] Add failing test to demonstrate parse error when @ is present in the description

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      This issue is created automatically through a Github pull request on behalf of Seldaek:

      Url: https://github.com/doctrine/common/pull/195

      Message:

      If someone can take this and fix the issue it'd be great. I couldn't figure it out at a quick glance and I don't really have time, but it's a pretty messed up bug and not so trivial to debug if you don't know what happens in the background so I'd say it's pretty important.

        Activity

        Hide
        Philipp Scheit added a comment - - edited

        I digged a little deeper. The test case is a great one
        The DocLexer cuts ut -1 position from the first @ (which is the @ in the @example.com). so that the lexer has to process:

        o@example.com"
             * }
             *
             * @AnnotationTargetPropertyMethod("Bar")
        

        So that the catchable pattern: ("(?:[^"]|"")*") matches here to greedy (it matches string in quotes):

        "
             * }
             *
             * @AnnotationTargetPropertyMethod("
        

        As a result the lexer does not catch the correct @ from the Annotation

        I could not think of a fast fix for this. But maybe tomorrow.
        (leaving quoted string pattern with an @ was a wrong idea)

        Its not a workaround to do more or less cutting in the parser, because not-well-formed quoted strings would break anyway

        Show
        Philipp Scheit added a comment - - edited I digged a little deeper. The test case is a great one The DocLexer cuts ut -1 position from the first @ (which is the @ in the @example.com). so that the lexer has to process: o@example.com" * } * * @AnnotationTargetPropertyMethod( "Bar" ) So that the catchable pattern: ("(?: [^"] |"")*") matches here to greedy (it matches string in quotes): " * } * * @AnnotationTargetPropertyMethod(" As a result the lexer does not catch the correct @ from the Annotation I could not think of a fast fix for this. But maybe tomorrow. (leaving quoted string pattern with an @ was a wrong idea) Its not a workaround to do more or less cutting in the parser, because not-well-formed quoted strings would break anyway
        Hide
        Philipp Scheit added a comment -

        can someone think of a case, where a string (in some annotation) has a newline in it? Otherwise the tests do not fail, when I leave the string matching pattern with a newline.
        I still had another idea to restrict the position of a annotation more strictly to:

        • the beginning of a new line (\s**\s* before)
        • in another annotation

        but the second is not solvable with the lexer itself and would transfer the quoted string matching to the parser, because it needs context.

        I'll let seldaek pull, what I have (is this the right way?)

        Show
        Philipp Scheit added a comment - can someone think of a case, where a string (in some annotation) has a newline in it? Otherwise the tests do not fail, when I leave the string matching pattern with a newline. I still had another idea to restrict the position of a annotation more strictly to: the beginning of a new line (\s**\s* before) in another annotation but the second is not solvable with the lexer itself and would transfer the quoted string matching to the parser, because it needs context. I'll let seldaek pull, what I have (is this the right way?)
        Show
        Marco Pivetta added a comment - PR @ https://github.com/doctrine/annotations/pull/17
        Hide
        Doctrine Bot added a comment -

        A related Github Pull-Request [GH-195] was closed:
        https://github.com/doctrine/common/pull/195

        Show
        Doctrine Bot added a comment - A related Github Pull-Request [GH-195] was closed: https://github.com/doctrine/common/pull/195
        Hide
        Guilherme Blanco added a comment -

        This fix was merged in latest master

        Show
        Guilherme Blanco added a comment - This fix was merged in latest master

          People

          • Assignee:
            Guilherme Blanco
            Reporter:
            Benjamin Eberlei
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: