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

[EJBCLIENT-178] Fix EJB JNDI parsing to be able to handle the case where an empty distinct name is not included in the JNDI name #180

Merged
merged 1 commit into from
Dec 20, 2016

Conversation

fjuma
Copy link
Contributor

@fjuma fjuma commented Dec 20, 2016

…ere an empty distinct name is not included in the JNDI name
if (size == 3) {
// name is of the form appName/moduleName/distinctName
return new RelativeContext(new FastHashtable<>(getEnvironment()), this, SimpleName.of(name));
}
Copy link
Member

Choose a reason for hiding this comment

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

I think that this suffers from a problem where you cannot disambiguate between app/module/distinct vs app/module/bean, doesn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Javadoc in the legacy EjbJndiNameParser indicates that a name of the form a/b/c will be considered to be a partial app/module/distinct lookup as opposed to a lookup of a bean name without a view. See https://github.com/jbossas/jboss-ejb-client/blob/3.x/src/main/java/org/jboss/ejb/client/naming/ejb/EjbJndiNameParser.java#L40-L42.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry if I'm just being dense here, but the existing code already behaves this way doesn't it? Any name with less than four segments is returned as a relative context, otherwise the four-segment name is decoded. AFAICT the existing code has no provision to convert a three-segment name into a bean w/o view.

Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to work out now if there's some case where the bean name is null where it wasn't meant to be; I guess that must be what's happening?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, any name with less than four segments is returned as a relative context. However, the problem with this is that if an empty distinct name has not been explicitly included in the lookup name, the current code incorrectly returns a relative context (since the lookup name will have three segments). The legacy parser was able to detect when an empty distinct name was left out of a lookup name.

This PR ensures that if a lookup name has three segments but the last segment includes a bean name, then we treat the distinct name as empty, just as the legacy parser would have done. Otherwise, if a lookup name has three segments but the last segment does not include a bean name, then the last segment must be a non-empty distinct name (and in this case, we should return a relative context).

Copy link
Member

Choose a reason for hiding this comment

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

Ah I was interpreting you backwards: I read that we did not want to support three segments under any circumstances. But if this was the legacy behavior then we definitely do need to preserve it.

@dmlloyd dmlloyd merged commit 323249e into wildfly:master Dec 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants