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

Handle nested brackets in a map keys [SPR-1274] #5976

Closed
spring-projects-issues opened this issue Aug 31, 2005 · 11 comments
Closed

Handle nested brackets in a map keys [SPR-1274] #5976

spring-projects-issues opened this issue Aug 31, 2005 · 11 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Aug 31, 2005

Christian Nelson opened SPR-1274 and commented

If the map key contains nested brackets, the key is not properly extracted from the string by BeanWrapperImpl.getPropertyNameTokens(String propertyName).

For example, consider these 'paths':

  1. "entries[some text[abc]]", key = "some text[abc" (missing ])
  2. "entries['some text[abc]']", key = "'some text[abc" (missing ]')
  3. "entries["some text[abc]"]", key = ""some text[abc" (missing ]")

The first one is kinda a stretch, and I didn't expect it to work. My natural reaction was to try the second and third, thinking that grouping it would do the trick. It didn't though.

This came up in the web application I'm working on. There's a form with let's the user assign IDs to textual names. The name sometimes contain brackets, which triggered this issue.

BeanWrapperImpl.java:610 (1.2.4 source) is the cause. The parser could count braces, and return the key when it's found the first matching pair. In the example above, the first ] would be part of the key, and the second, which completes the initial pair, would complete the key. This solution would work regardless of the single or double quotes. An alternative would be to ignore braces that are in quoted ections of the string. This naturally would only work for scenarios #2 and #3 above.

I will try to update BeanWrapperImpl.java with a sllightly more sophisticated bracket parser and submit a patch with tests. If you think it's a waste of time, or if there's something obvious I'm missing, feel free to point out my error. Regards, Christian


Affects: 1.2.4

Attachments:

Issue Links:

5 votes, 4 watchers

@spring-projects-issues
Copy link
Collaborator Author

Christian Nelson commented

There is a forum thread associated with this issue as well: http://forum.springframework.org/viewtopic.php?t=8468

@spring-projects-issues
Copy link
Collaborator Author

Christian Nelson commented

This patch (spr-1274-patch-1.txt) contains my update to BeanWrapperImpl.java, and the associated unit tests). It's in eclipse's universal format.

@spring-projects-issues
Copy link
Collaborator Author

Christian Nelson commented

I should mention that the patch resolves the issue in all three scenarios listed in the original bug report.

@spring-projects-issues
Copy link
Collaborator Author

Christian Nelson commented

The first patch missed a necessary updat to addStrippedPropertyPaths(...). This caused the lookup of custom editors to fail.

This new patch (spr-1274-patch-2.txt) includes the first patch, as well as the additional changes.

The file MapBindingTests.java includes the unit test used to diagnose the custom editor lookup problem. It may be interestnig to review or possibly add to the test suite.

@spring-projects-issues
Copy link
Collaborator Author

Oliver Hutchison commented

What if the key has and unmatched brace? For example "entries[[java.lang.String]".

IMHO what we realy need is a way to escape the brace.

@spring-projects-issues
Copy link
Collaborator Author

Christian Nelson commented

Oliver, apparently, sometimes I miss the obvious...

I think I'll tweak my patch so that brackets in string literals aren't processed. I think that will do the trick. Excaping would also work, but I'd like to find a way to automate it or something, which I need to investigate.

Thanks for the comment!
Christian

@spring-projects-issues
Copy link
Collaborator Author

Alex commented

Hi there,

I am using Spring 1.2.7 and this issue is affecting me. I tried to patch using the attached patch but it didn't seem to patch cleanly. Is there an updated patch for 1.2.7 or the current production release (1.2.8) somewhere, so that brackets in string literals aren't processed?

I see the target is 2.1RC1 but it would be useful to backport this fix.

Thanks

Alex

@spring-projects-issues
Copy link
Collaborator Author

Christian Nelson commented

Alex,

Unfortunately, that patch didn't fully fix the problem. There were some test cases that still failed and I was never able to develop a complete fix.

Oliver is right is saying the right fix is to implement some sort of excaping. The bracket counting I was working on is problematic and obscure.

Vote for the issue and then perhaps it will catch the Spring team's attention once again.

Christian

@spring-projects-issues
Copy link
Collaborator Author

Alex commented

I understand that the existing patch does not support some scenarios (i.e. unmatched brackets) without escaping, but nevertheless it is a better solution to handling brackets than the stock approach?

Would this not justify the release of a 'working' patch, even if it only supported matching bracket pairs?

In any case, surely there is a problem with escaping solution, in that (as far as I know) it is not a part of the javabeans standard property notation? Or would it be possible to overlook this...

Thanks

Alex.

@spring-projects-issues
Copy link
Collaborator Author

Alon Salant commented

I am also experiencing this problem in Spring 2.0.8.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Closing groups of outdated issues. Please reopen if still relevant.

@spring-projects-issues spring-projects-issues added status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) labels Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants