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

Add detection of HCL Digital Experience rebranding #12242

Merged
merged 3 commits into from
Aug 9, 2021
Merged

Add detection of HCL Digital Experience rebranding #12242

merged 3 commits into from
Aug 9, 2021

Conversation

vjt
Copy link
Contributor

@vjt vjt commented Mar 12, 2021

Dear Vaadin team,

On HCL Digital Experience 8.5.5 CF19, the getPortalInfo() method returns "hcl digital experience/8.5", breaking detection of the servlet engine. This ultimately leads to methods such as getHeader() to return NULL, as the upstream HTTP request is not retrieved by the Vaadin abstraction layer.

In our case, the issue became evident with the MultiFileUploader components, that could not read the Content-Length header contents.

Below a debugging session screenshot demonstrating the issue.

Thank you for your consideration.

hcl-portal-vaadin-debug-session


This change is Reviewable

On HCL Digital Experience 8.5.5 CF19, the `getPortalInfo()` method returns
"hcl digital experience/8.5", breaking detection of the servlet engine.

This ultimately leads to methods such as `getHeader()` to return NULL, as
the upstream HTTP request is not retrieved.
@CLAassistant
Copy link

CLAassistant commented Mar 12, 2021

CLA assistant check
All committers have signed the CLA.

@DanieleVistalli
Copy link

I confirm the issue. HCL Software acquired the former WebSphere Portal product.

As part of the rebranding the portalInfo string changed and the string based detection is failing. This has impact on any customer updating to HCL DX 8.5 and 9.X with CF19 (Cumulative fix 19)

@TatuLund
Copy link
Contributor

Yes, this seems to be already quite old news actually, ref. https://www.hcltech.com/software/acquisition-updates

@TatuLund
Copy link
Contributor

@vjt The change looks legit, but have you tested it someway. Luckily createVaadinRequest is protected so you can override that with your own implementation that does the same thing as your fix.

Just pointing out that there is WebSphere Portlet specific code here too: https://github.com/vaadin/framework/blob/master/server/src/main/java/com/vaadin/server/VaadinPortlet.java#L330

Assuming that HCL has not done any actual code level changes, e.g. package renaming, you proposed change could be enough.

@vjt
Copy link
Contributor Author

vjt commented Mar 17, 2021

Hi @TatuLund,

I have tested Vaadin works correctly on a CF19 portal by modifying the portal identity string, so that it returns again ibm websphere portal, by unarchiving PortalServer/base/wp.base/shared/app/wp.base/wp.base.jar and editing com/ibm/wps/Version.properties and changing server.name.

Internally we are moving in the direction you pointed out, overriding the Vaadin method and applying this change - I anyway believe it would be valuable for Vaadin to support Portal CF19.

Eventually - my personal opinion is that HCL should revert this change, as I do not understand breaking software integration compatibility if there are no technical reasons to do so, and without a widespread "breaking change" announcement.

Thanks

@DanieleVistalli
Copy link

@vjt The change looks legit, but have you tested it someway. Luckily createVaadinRequest is protected so you can override that with your own implementation that does the same thing as your fix.

Just pointing out that there is WebSphere Portlet specific code here too: https://github.com/vaadin/framework/blob/master/server/src/main/java/com/vaadin/server/VaadinPortlet.java#L330

Assuming that HCL has not done any actual code level changes, e.g. package renaming, you proposed change could be enough.

Package renaming didn't happen and is not going to happen (it would break a lot more than Vaadin)

I talked to HCL development and they confirmed the string change due to IBM to HCL rebranding.

The code fix in this PR will make it. If you need a confirmation I can have my team do/test it but it just looks like the fix is just correct.

@TatuLund TatuLund added this to the 8.13.0 milestone Mar 17, 2021
@TatuLund
Copy link
Contributor

@DanieleVistalli Thanks for quick feedback and we appreciate your contribution on this. We simply cannot keep our eyes on everything that moves, and in Java ecosystem there are so many things that moves.

I can have my team do/test it but it just looks like the fix is just correct.

This would be of course be something that would be greatly appreciated. The code change is neutral in the sense that it wont break anything, but I would assume it would be also nicer for you to get update that surely works, rather than need to re-patch and wait again for the next release.

@DanieleVistalli
Copy link

I will test this shortly. And provide confirmation by Monday next week

@TatuLund
Copy link
Contributor

@DanieleVistalli Did you manage to test it?

@ZheSun88 ZheSun88 removed this from the 8.13.0 milestone Apr 20, 2021
@TatuLund
Copy link
Contributor

TatuLund commented May 4, 2021

@DanieleVistalli did you have some outcome from your testing?

@Ansku Ansku merged commit 5dd5cdc into vaadin:master Aug 9, 2021
@vjt vjt deleted the patch-1 branch August 9, 2021 14:08
@vjt
Copy link
Contributor Author

vjt commented Aug 9, 2021

🎉

Ansku pushed a commit that referenced this pull request Aug 11, 2021
On HCL Digital Experience 8.5.5 CF19, the `getPortalInfo()` method returns
"hcl digital experience/8.5", breaking detection of the servlet engine.

This ultimately leads to methods such as `getHeader()` to return NULL, as
the upstream HTTP request is not retrieved.
Ansku pushed a commit that referenced this pull request Aug 11, 2021
On HCL Digital Experience 8.5.5 CF19, the `getPortalInfo()` method
returns "hcl digital experience/8.5", breaking detection of the servlet
engine.

This ultimately leads to methods such as `getHeader()` to return NULL,
as the upstream HTTP request is not retrieved.
Ansku added a commit that referenced this pull request Aug 11, 2021
On HCL Digital Experience 8.5.5 CF19, the `getPortalInfo()` method
returns "hcl digital experience/8.5", breaking detection of the servlet
engine.

This ultimately leads to methods such as `getHeader()` to return NULL,
as the upstream HTTP request is not retrieved.

Authored-by: Marcello Barnaba <vjt@openssl.it>
@Ansku Ansku added this to the 8.13.3 milestone Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants