Skip to content

OGNLContext deprecated constructor crashes #81

Closed

Description

@deved65

In current release 3.2.11 and master the deprecated constructor public OgnlContext(Map values) in line 117 is unusable and should be removed immediately (or fixed). Currently it will always leads to a NullPointerException in line 170 of the class. To my understanding functionality marked with 'deprecated' should no longer be used (as it will be removed in a future release), but should not be unusable 'by concept'.

Activity

  1. lukaszlenart commented on Sep 25, 2019

    @lukaszlenart
    Collaborator

    Are you sure it's about the line 170?

  2. deved65 commented on Sep 26, 2019

    @deved65
    Author

    Sorry, I messed it up. It is line 140 with a RuntimeException. In line 120 the extensive constructor is invoked with null reference for parameter memberAccess. In line 137 is a null reference check on memberAccess which in turn always must lead to the RuntimeException.

  3. lukaszlenart commented on Sep 26, 2019

    @lukaszlenart
    Collaborator

    The best option I see is to drop that constructor..

  4. deved65 commented on Sep 28, 2019

    @deved65
    Author

    If no meaningful or 'cheap' default implementation for MemberAccess can be provided I tend to agree.

  5. lukaszlenart commented on Sep 28, 2019

    @lukaszlenart
    Collaborator

    Yeah... that's why I have removed the default implementation from the main code and moved it into tests - it was quite often used to replace a framework's specific implementation by hackers. And that's why OGNL 3.2.x requires to provide your own implementation, even as a copy of the default one based on the one used in tests.

  6. lukaszlenart commented on Oct 3, 2019

    @lukaszlenart
    Collaborator

    Also please be aware that 3.2.x line is still a work-in-progress - maybe I should express that better

  7. JCgH4164838Gh792C124B5 commented on Feb 8, 2020

    @JCgH4164838Gh792C124B5
    Contributor

    Hi.

    Dropping the single-argument constructor: https://github.com/jkuhnert/ognl/blob/6de4049f093eb643332bbf14090006fac516393c/src/main/java/ognl/OgnlContext.java#L121 that will now always fail with an exception seems reasonable to me.

    At the same time, would it be beneficial to replace the generic RuntimeException exception: https://github.com/jkuhnert/ognl/blob/6de4049f093eb643332bbf14090006fac516393c/src/main/java/ognl/OgnlContext.java#L144
    with a more specific IllegalArgumentException instead ?

    The updated exception could be something like:

    throw new IllegalArgumentException("MemberAccess implementation must be provided - null not permitted!");
    
  8. lukaszlenart commented on Feb 23, 2020

    @lukaszlenart
    Collaborator

    Good idea, if no objections let's do it :)

  9. added a commit that references this issue on Feb 24, 2020
  10. lukaszlenart commented on Mar 12, 2020

    @lukaszlenart
    Collaborator

    OGNL 3.2.14 is out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions