search

eclipse-birt / birt

Eclipse BIRT™ The open source reporting and data visualization project.
http://www.eclipse.org/birt
Eclipse Public License 2.0
423 stars 387 forks source link

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

Closed speckyspooky closed 8 months ago

speckyspooky commented 8 months ago

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

claesrosell commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

cell-span.zip

claesrosell commented 8 months ago

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 commented 8 months ago

This is how it looks in my setup. image

claesrosell commented 8 months ago

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

speckyspooky commented 8 months ago

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

claesrosell commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

jasper

claesrosell commented 8 months ago

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 commented 8 months ago

@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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

image

brianvfernandes commented 2 months ago

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 commented 2 months ago

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