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

Use standard String comparison in ExtendedBeanInfo.PropertyDescriptorComparator #31866

Closed
scqiu opened this issue Dec 20, 2023 · 1 comment
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@scqiu
Copy link

scqiu commented Dec 20, 2023

Affects: 5.3.20


The compare method of ExtendedBeanInfo.PropertyDescriptorComparator class

public int compare(PropertyDescriptor desc1, PropertyDescriptor desc2) {
			String left = desc1.getName();
			String right = desc2.getName();
			byte[] leftBytes = left.getBytes();
			byte[] rightBytes = right.getBytes();
			for (int i = 0; i < left.length(); i++) {
				if (right.length() == i) {
					return 1;
				}
				int result = leftBytes[i] - rightBytes[i];
				if (result != 0) {
					return result;
				}
			}
			return left.length() - right.length();
		}

For some string encode of utf-8, the length of left is not equal to the length of leftBytes.

The for-loop should use the length of leftBytes.

For example if the left is "指标大类", the right is "指标名称". This method will return 0. Obviously it's wrong.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 20, 2023
@jhoeller
Copy link
Contributor

Good catch! We only sort Java property descriptor names there for some basic order when returning them from BeanInfo.getPropertyDescriptors(), so it's arguably not a bug in that context, but the algorithm is questionable even there.

It turns out that the entire implementation seems to mimic String comparison behavior. We can simplify that to return desc1.getName().compareTo(desc2.getName()) and avoid that manual algorithm to begin with, addressing any encoding problems along with that. I'll repurpose this GitHub issue for that revision in 6.1.3.

@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 20, 2023
@jhoeller jhoeller self-assigned this Dec 20, 2023
@jhoeller jhoeller added this to the 6.1.3 milestone Dec 20, 2023
@jhoeller jhoeller changed the title a bug of ExtendedBeanInfo.PropertyDescriptorComparator class Use standard String comparison in ExtendedBeanInfo.PropertyDescriptorComparator Dec 20, 2023
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) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants