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

BIRT 4.14: After "Jetty 12" migration the web-viewer of the HTML-preview is crashed in design and functionality #1466

Closed
speckyspooky opened this issue Oct 30, 2023 · 27 comments · Fixed by #1473
Assignees
Labels
BugFix Change to correct issues
Milestone

Comments

@speckyspooky
Copy link
Contributor

speckyspooky commented Oct 30, 2023

Hello @claesrosell & @merks

I started directly after the master update the update of my fork and the result is mixed situation.
The good thing is we can start from eclipse-designer now the output-creation like xlsx, pdf, etc.

But the BIRT-viewer is crashed and I'm not sure is it an issue on my side with the project update or is there a topic with the Jetty-migration. What is the result if you start the BIRT-viewer on your eclipse version?

BIRT-viewer based on the latest changes

BIRT4 14-issue-viewer-not-usable

Console of missing osgi-component:

Unresolved requirement: Require-Bundle: org.eclipse.jst.ws.axis.consumption.core; bundle-version="[1.1.0,2.0.0)"

osgi-axis-issue

View into the browser console

osgi-axis-issue-browser

@speckyspooky speckyspooky added the BugFix Change to correct issues label Oct 30, 2023
@speckyspooky speckyspooky added this to the 4.14 milestone Oct 30, 2023
@claesrosell
Copy link
Contributor

claesrosell commented Oct 30, 2023

My guess is that the OSGI BundleException is because axis 1.4.0 and axis.ant 1.4.0 isn't selected in the debug configuration. (.launch file)
The other errors (from the Chrome DevTools ), I cannot not really comprehend. I did not encounter them in my, pretty small, test. Can you attach your test - .rptdesign ?

@merks
Copy link
Contributor

merks commented Oct 30, 2023

I need to find time in the coming days to look closely at the current state. I hope later in this week.

@claesrosell Did any test get disabled because of the circularity issue?

@claesrosell
Copy link
Contributor

I need to find time in the coming days to look closely at the current state. I hope later in this week.

@claesrosell Did any test get disabled because of the circularity issue?

No tests were disabled. I removed the circular dependency instead.

@speckyspooky
Copy link
Contributor Author

Currently I have the effect on every report. Here is my report of my xlsx-fix.

cell-span.zip

@claesrosell
Copy link
Contributor

It looks like I missed updating one Jetty entry in the birt feature. It still points to 10.0.15 which can of cause create problems.

@claesrosell
Copy link
Contributor

This is how it looks in my setup.
image

@claesrosell
Copy link
Contributor

Forget about what I wrote about the 10.0.15 version in the feature. I looked at the wrong place.

@speckyspooky
Copy link
Contributor Author

The viewer looks correct on your side.
Yes, this is my small test report of the excel topic.

@claesrosell
Copy link
Contributor

Can you confirm that you have run the "Perform Setup Tasks -> Modular Target" thing? I do not know if that one updates the debug configuration as well, but probably not. You will need to make sure that the axis 1.4.0 and axis.ant 1.4.0 is selected. To be sure, also check the jetty plugins and check out any 10.0.15 bundles if you still have such bundles in your bundle pool. After that you should be able to do a "Select Required" to add any missing required bundles.

@speckyspooky
Copy link
Contributor Author

I have checked the plugins on the run configuration and added the axis-part (axis 1.4.0 and axis.ant 1.4.0) but without success.
For me it is not clear what you mean with "Perform Setup Tasks -> Modular Target".

The jetty 10.0.15 bundles are noted as missing on the plugin validation. So the plugins are marked like required but will be missed on my side. Do you mean this can cause the issue.

axis-plugins

axis

jetty missing

jetty-10

@claesrosell
Copy link
Contributor

Yes. You shouldn't have any jetty 10 bundles selected in your debug configuration.
Guessing from your second screenshot I would say that org.eclipse.jetty.http, org.eclipse.jetty.security, org.eclipse.jetty.server and org eclipse.jetty.servlet are all active with version 10.0.15 in the debug configuration. Remove those. Their 12.0.2 versions should be selected instead.

@speckyspooky
Copy link
Contributor Author

I have removed all 10.0.15 jetty plugins. The validation of the plugins is fine but the viewer starts like before as the crashed-version.

@claesrosell
Copy link
Contributor

Which versions of org.mortbay.jasper.apache-el and org.mortbay.jasper.apache-jsp are active in the debug configuration? They need to be "9.0.52" but the generated target platform has been known to pull newer versions.

@speckyspooky
Copy link
Contributor Author

Both jasper-libraries have the version "9.0.52" on my side.

jasper

@claesrosell
Copy link
Contributor

I will fire up a dev environment on my windows machine and see if I can reproduce the problem there. Jetty has some security settings that prevents access to files that uses::.... paths , which has caused problems in dev-enviroment before. Back then it worked in Windows but nor in Linux though.

@claesrosell
Copy link
Contributor

@speckyspooky I get the same error as you when I am running from my windows dev environment. I'll be back with more information soon.

@claesrosell
Copy link
Contributor

So, the problem was indeed, or at least in part, relative paths, or "alias" as Jetty calls them.

Can you test this in your dev-environment @speckyspooky :

In ViewerWebApp.java, line 58 and 59 switch:

	URL resolvedWebAppUrl = FileLocator.resolve(webAppUrl);
	URL resolvedWebDescriptorUrl = FileLocator.resolve(webDescriptorUrl);

to

	URI resolvedWebAppUrl = FileLocator.resolve(webAppUrl).toURI().normalize();
	URI resolvedWebDescriptorUrl = FileLocator.resolve(webDescriptorUrl).toURI().normalize();

... and see if that helps. I think it will since it solved the problem on my end.

@speckyspooky
Copy link
Contributor Author

Alright, @claesrosell you are the "master of the day".
I changed the 2 lines and now the preview is running normally 💯

I understand you change. But do you know wich change had such (strange) effect to change URL-to-URL and therefore we got the issue?

@claesrosell
Copy link
Contributor

If you want to understand why this solution "works" you can change back those lines and set a breakpoint in org.eclipse.jetty.ee8.nested.ContextHandler.checkAlias() . You will see that Jetty refuses the request to resources which are not direct. This includes symlinks and "relative" directories.

The magic here is the .normalize() call, which does not exist on URL, hence the conversion to URI and back.

What I really would like to do is use an URLResource to the bundleentry as baseResource on the WebAppContext. That used to work in Jetty-10 but does not work any longer. I suspect that line 650 in org.eclipse.jetty.server.handler.ContextHandler

      if (!Resources.isReadable(baseResource))

is wrong. Think that it should be

      if (!Resources.isReadableDirectory(baseResource))

instead. I have not checked with the Jetty guys though.

@brianvfernandes
Copy link

@claesrosell the URL<>URI fix breaks report preview (not sure what else) in installs with a space in the path, at least on Windows. Can be reproduced with the BIRT 4.15 All-in-one distribution.

  1. Unzip BIRT all-in-one to a location which has a space in the path.
  2. Create a report project and a new report - even a blank report will do.
  3. Use the preview button on the toolbar to, "View Report as PDF", "View Report in Web Viewer", etc.
  4. You should see errors in the log around not being able to start the application server, the crux of it is that the URL to URI conversion will fail if there's a space in the path:

java.net.URISyntaxException: Illegal character in path at index 34: file:/C:/EclipseInstalls/birt-4.15 AIO/eclipse/plugins/org.eclipse.birt.report.viewer_4.15.0.v202403270652/birt/
at java.base/java.net.URI$Parser.fail(URI.java:2995)
at java.base/java.net.URI$Parser.checkChars(URI.java:3166)
at java.base/java.net.URI$Parser.parseHierarchical(URI.java:3248)
at java.base/java.net.URI$Parser.parse(URI.java:3196)
at java.base/java.net.URI.(URI.java:645)
at java.base/java.net.URL.toURI(URL.java:1220)
at org.eclipse.birt.report.viewer.utilities.ViewerWebApp.start(ViewerWebApp.java:59)

Workspace error log

@hvbtup
Copy link
Contributor

hvbtup commented Apr 3, 2024

A bit OT, but...

If using an installation path without spaces is a workaround, then I wonder if fixing this is worth the effort.

Those who use spaces in directory names (at least on Windows) are looking for trouble.
This is one thing developers on Windows learn pretty soon.
There's a reason why e.g. the directory is called c:\users and no longer "C:\documents and settings".

OTOH, errors like this always smell like a security issue (missing or wrong or double escaping when writing or parsing data).

@claesrosell
Copy link
Contributor

claesrosell commented Apr 3, 2024

Hi.
I am currently on vacation and do not have a computer available. I will try look into this when I am back, next week. In the the meantime, please create a new Issue for this problem.

@merks
Copy link
Contributor

merks commented Apr 3, 2024

I had a quick look but it's super annoying that one gets back a URL from the framework but then it turns out a valid URL is not necessarily a valid URI. And, instead of encoding a space when calling toURI, let's just throw an exception. Go figure that out...

@speckyspooky
Copy link
Contributor Author

I'm with Ed, yes we could check if an unescaped "space" is given into the URL and we can escape it.
At the end we would handle 1 special character ("ok" a very standard character) and no further one.

@merks
Copy link
Contributor

merks commented Apr 3, 2024

The org.eclipse.core.runtime.URIUtil.toURI(URL) method deals with such nastiness:

image

@brianvfernandes
Copy link

Good to know about URIUtil - I half wrote a small variant of the above, far more formidable method.
Truly fantastic how fast this was addressed - great work @speckyspooky and @merks thank you!

@speckyspooky
Copy link
Contributor Author

The latest nigthly build looks good. The 2 PRs #1621 & #1629 solve the topic with the space on the designer path.

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