eclipse-embed-cdt / eclipse-plugins

The Eclipse Embedded CDT plug-ins for Arm & RISC-V C/C++ developers (formerly known as the GNU MCU Eclipse plug-ins). Includes the archive of previous plug-ins versions, as Releases.
http://eclipse-embed-cdt.github.io/
Eclipse Public License 2.0
555 stars 129 forks source link

Parsing Keil index.pidx fails due to redirection to https #373

Closed flynneva closed 4 years ago

flynneva commented 4 years ago

Description

I am having similar problems as the symptoms described on this issue #39....with a specific error being printed out in the console.

Eclipse Error shown after refresh:

org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 50; White spaces are required between publicId and systemId.

It looks like the downloaded file might have a typo in the first line but I am not sure? Eclipse console log:

2020-01-15 13:22:09 Update packs job started. Parsing "http://www.keil.com/pack/index.pidx"... org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 50; White spaces are required between publicId and systemId. File "/home/continental/opt/cmsis-packs/.cache/.content_www_keil_com_pack_index_pidx.xml" written.

Update packs completed in 0s.

2020-01-15 13:22:10 Loading repos summaries... Parsing cached content file "/home/continental/opt/cmsis-packs/.cache/.content_www_keil_com_pack_index_pidx.xml"... File parsed in 7ms. Identifying installed packages... Found no installed packages. Completed in 8ms.

2020-01-15 13:22:10 Collecting devices... Found none. Completed in 1ms.

2020-01-15 13:22:10 Collecting packs... Found none. Completed in 1ms.

Steps to Reproduce

  1. Set your MCU Packages URL
  2. Hit refresh in the Packs panel
  3. Read out console

Expected behaviour: Packs downloads list of packages and parses it into a list

Actual behaviour: Error on parsing step, line 1 column 50 of index.pidx file

Versions

ilg-ul commented 4 years ago

I can reproduce the issue on my computer.

I'll investigate what is wrong with the Keil index.

ilg-ul commented 4 years ago

https://github.com/ARM-software/CMSIS_5/issues/800

ilg-ul commented 4 years ago

As a workaround, you can copy the index locally and parse it from there, the problem seems to affect only direct URL parsings.

ilg-ul commented 4 years ago

Further tests show a possible problem with the new Keil server, which does not report the content type properly.

I made a copy of the index to GitHub, you can try to use it from

https://github.com/ilg-ul/test-sax-validation/raw/master/index.pidx

As suggested bellow, a better URL would be the redirected Keil one:

https://sadevicepacksprodus.blob.core.windows.net/idxfile/index.pidx

truhlikfredy commented 4 years ago

I tested it now and your URL works

ilg-ul commented 4 years ago

Thank you, Anton.

Let's see if Keil/Arm will find a way to fix their servers.

truhlikfredy commented 4 years ago

Surprised they didn't tested it more, as redirects are notorious to break a lot of stuff.

ilg-ul commented 4 years ago

There must be a reason why they decided, in their plug-ins, to copy the files locally and parse them from there, don't you think?

cdwilson commented 4 years ago

For those running into this issue, a quick workaround is to change the URL in the .repos.xml file in the packages directory to the redirected URL

i.e. change this

<?xml version="1.0" encoding="UTF-8"?>
<repositories>
  <repository>
    <type>CMSIS Pack</type>
    <name>Keil</name>
    <url>http://www.keil.com/pack/index.pidx</url>
  </repository>
</repositories>

to this:

<?xml version="1.0" encoding="UTF-8"?>
<repositories>
  <repository>
    <type>CMSIS Pack</type>
    <name>Keil</name>
    <url>https://sadevicepacksprodus.blob.core.windows.net/idxfile/index.pidx</url>
  </repository>
</repositories>
ilg-ul commented 4 years ago

You don't have to manually edit that file, there is a GUI preference for managing the repo URLs in C/C++ -> MCU Packages -> Repositories.

truhlikfredy commented 4 years ago

This preference page: repo

ilg-ul commented 4 years ago

A quick update, the issue is caused by the Java class HttpURLConnection, which in certain cases does not follow redirects.

To make things worse, a small test program using this class with the Keil URL worked as expected.

I don't know why the behaviour is different when used inside Eclipse.

This class has a way to control if redirects are followed, and initially I thought that in Eclipse this setting is configured differently, but after checking it, it proved to be set as expected.

I'll further investigate and try to find a workaround.

ilg-ul commented 4 years ago

In early January 2020 Keil switched from http to https, and apparently the Java classes are not able to automatically process such redirects, which must be handled manually in the code.

As a better workaround, please update the index url from http://www.keil.com/pack/index.pidx to https://www.keil.com/pack/index.pidx and let me know if this worked for you too.

ilg-ul commented 4 years ago

I fixed the code to follow redirects from http to https.

A pre-release of 4.7.2 is available for testing from

http://gnu-mcu-eclipse.netlify.com/v4-neon-updates-test

Please let me know if this works as expected.

cdwilson commented 4 years ago

Changing the repository URL to https in Eclipse preferences fixed this issue for me 👍

ilg-ul commented 4 years ago

Please try the new release, it sould work with http too.

cdwilson commented 4 years ago

Yes, new release works with http

truhlikfredy commented 4 years ago

Did the small test program run different java runtime? I think the networking was changed a bit in the later java runtime versions.

ilg-ul commented 4 years ago

Did the small test program run different java runtime?

Yes.

Unfortunately the small test was done with https, which worked. I retried it with http and it failed to follow the redirection, so the Java class behaviour is consistent.

truhlikfredy commented 4 years ago

When thinking about it, it might have been a completely new functionally, maybe I recalled this: https://openjdk.java.net/groups/net/httpclient/intro.html

While the original functionally is probably untouched as much as they are able to

ilg-ul commented 4 years ago

Yeah, who knows, things seem very complicated.

I already implemented redirection in my code, so no need to further worry about this.

flynneva commented 4 years ago

Looks like this was solved, is there a PR we could reference to get the fix into the main line and in the next release?

ilg-ul commented 4 years ago

The commits mention this issue number and are automatically linked to this, see above.

The new binaries are already available, you can install new software from the updates-test link.

I'll make the new release these days.