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

Update Maplibre to version 5 #924

Merged
merged 2 commits into from
Jan 31, 2025
Merged

Update Maplibre to version 5 #924

merged 2 commits into from
Jan 31, 2025

Conversation

YongGoose
Copy link
Contributor

Resolves #922

This PR updates the Maplibre library to version 5.0.1.

@bchapuis
Copy link
Member

Thanks a lot for this contribution.

I have tried this change with the dev and serve command of the baremaps CLI and got some Content Security Policy issues in the browser. After digging a bit, I managed to display the map by updating the value of the integrity attribute for the js and css files of maplibre and by adding a meta element at the top of the head section to declare the CSP.

Here are my changes:

  <meta http-equiv="Content-Security-Policy" content="
    script-src 'self' 'unsafe-eval' 'unsafe-inline' https://unpkg.com https://baremaps.apache.org;
    worker-src 'self' blob:;
    child-src 'self' blob:;
    img-src 'self' data: blob: https://unpkg.com https://baremaps.apache.org;
    style-src 'self' 'unsafe-inline' https://unpkg.com https://baremaps.apache.org;
  ">

  <script src="https://unpkg.com/[email protected]/dist/maplibre-gl.js" integrity="sha256-VkgYz8vlPsIndJcxwEDZKAdx4r+Ag7HcLBPP4UbJrZE=" crossorigin="anonymous"></script>
  <script src="https://baremaps.apache.org/assets/maplibre/maplibre-gl-inspect.js" integrity="sha256-NYdRoIbqeAWkTHjpa/BukMLXcsiqFoDuJCYzzaRei30=" crossorigin="anonymous"></script>
  <script src="https://baremaps.apache.org/assets/maplibre/maplibre-gl-tile-boundaries.js" integrity="sha256-Jo8NvxMzooqayU8+eIsjO49b4EoakoE0o0tSSrBWPCU=" crossorigin="anonymous"></script>
  <script src="https://baremaps.apache.org/assets/maplibre/maplibre-custom-controls.js" integrity="sha256-80a3VIbp5OJ8HSIWJ7+6NjTvdBmVirLCR6otKucXnlw=" crossorigin="anonymous"></script>
  <script src="https://baremaps.apache.org/assets/maplibre/maplibre-gl-framerate.js" integrity="sha256-6IVzv2heNDAl7KkSqjRkmjr56asfkdCxO2q8ouTo8t8=" crossorigin="anonymous"></script>
  <link rel="stylesheet" href="https://unpkg.com/[email protected]/dist/maplibre-gl.css" integrity="sha256-eSrJl9z2rm9kPrTi3uRjDIXnBWUmvY+4X/6Dxn1sQbQ=" crossorigin="anonymous">
  <link rel="stylesheet" href="https://baremaps.apache.org/assets/maplibre/maplibre-gl-inspect.css" integrity="sha256-ePhsoaklgbCBxK7kfSUcP+V+bVqnfFwpILz7hkSR0Lo=" crossorigin="anonymous">
  <link rel="stylesheet" href="https://baremaps.apache.org/assets/maplibre/maplibre-gl-tile-boundaries.css" integrity="sha256-FxIRliWXnU67sT1i97MgNQ0qL8LjsgYiBH9v/uNxzdM=" crossorigin="anonymous">
  <link rel="stylesheet" href="https://baremaps.apache.org/assets/maplibre/maplibre-custom-controls.css" integrity="sha256-rFc19qrZTPgcrNeYjtsLCcDOqM5NrLiPy2U+0FxW9vA=" crossorigin="anonymous">
  <link rel="icon" type="image/x-icon" href="https://baremaps.apache.org/assets/favicon/favicon.ico">

Can you cross check and validate that it works for you as well?

@YongGoose
Copy link
Contributor Author

Can you cross check and validate that it works for you as well?

I'll test the provided changes and validate the results.
Get back to you with my findings by tomorrow :)

@YongGoose
Copy link
Contributor Author

@bchapuis

Sorry for the delay.
I went on a vacation with my family 😅


First, I have reflected the changes in this commit. 7be1623

After digging a bit, I managed to display the map by updating the value of the integrity attribute for the js and css files of maplibre and by adding a meta element at the top of the head section to declare the CSP.

In my case, even after updating the integrity values as in the commit above and adding the meta element, the issue was not resolved.

To share more details about the problem, I will attach a screenshot along with the commands I executed.

screenshot

image

commands

baremaps map serve --tileset tileset.js --style style.js

I ran the command in the Basemap directory

  • If I misunderstood or made any mistake, please feel free to let me know!

@bchapuis bchapuis self-requested a review January 29, 2025 14:09
Copy link
Member

@bchapuis bchapuis left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. The opening style element in server.html is missing. After this change both the dev and serve commands are working for me. Could you try to perform a hard refresh or to open the url in anonymous mode? I'm not sure why you get a integrity error.

<link rel="stylesheet" href="https://baremaps.apache.org/assets/maplibre/maplibre-gl-inspect.css" integrity="sha256-ePhsoaklgbCBxK7kfSUcP+V+bVqnfFwpILz7hkSR0Lo=" crossorigin="anonymous">
<link rel="stylesheet" href="https://baremaps.apache.org/assets/maplibre/maplibre-gl-tile-boundaries.css" integrity="sha256-FxIRliWXnU67sT1i97MgNQ0qL8LjsgYiBH9v/uNxzdM=" crossorigin="anonymous">
<link rel="stylesheet" href="https://baremaps.apache.org/assets/maplibre/maplibre-custom-controls.css" integrity="sha256-rFc19qrZTPgcrNeYjtsLCcDOqM5NrLiPy2U+0FxW9vA=" crossorigin="anonymous">
<link rel="icon" type="image/x-icon" href="https://baremaps.apache.org/assets/favicon/favicon.ico">
<style>
Copy link
Member

Choose a reason for hiding this comment

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

This style element is shouldn't be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 44aaacf !

@YongGoose
Copy link
Contributor Author

Thanks for the changes. The opening style element in server.html is missing. After this change both the dev and serve commands are working for me.

When opening a local file via a URL, the integrity error did not occur.
image

  • I confirmed that the integrity error happens when the integrity value is changed in the case above.

Could you try to perform a hard refresh or to open the url in anonymous mode? I'm not sure why you get a integrity error.

After clearing the cache and performing a hard refresh, the result was still the same.

However, I believe the issue lies with my development environment.
If it were a problem with the code, I think the same integrity error should occur when I open the local file through the URL.

WDYT?

@bchapuis
Copy link
Member

bchapuis commented Jan 29, 2025

I'm not completely sure why you get this error. On my side, it works for 127.0.0.1:9000 and localhost:9000. I will try to test it with a different browser and maybe ask someone else to test it. Overall, things look good to me.

@YongGoose
Copy link
Contributor Author

I'm not completely sure why you get this error. On my side, it works for 127.0.0.1:9000 and localhost:9000. I will try to test it with a different browser and maybe ask someone else to test it. Overall, things look good to me.

Thank you! 👍🏻
I'll try testing it with a different browser as well.

PS. The template is currently a work in progress, and I'll create a PR once it's completed.

@bchapuis
Copy link
Member

bchapuis commented Jan 31, 2025

@YongGoose I further investigated the content security policy and I think that the error was comming from a chrome extention that interfered with the policy. It does not show up in incognito mode.

I also tried to better understand why things were not working properly with localhost and 127.0.0.1 and I think it comes from the fact that they are not considered as the same by CSP. So here is a revised meta header works for all cases and is probably better. I also removed the eval-unsafe directive which was unecessary.

  <meta http-equiv="Content-Security-Policy" content="
    default-src 'self' http://127.0.0.1:* http://localhost:* https://unpkg.com https://baremaps.apache.org;
    script-src 'self' http://127.0.0.1:* http://localhost:* https://unpkg.com https://unpkg.com https://baremaps.apache.org 'unsafe-inline';
    worker-src 'self' blob:;
    child-src 'self' blob:;
    img-src 'self' data: blob: http://127.0.0.1:* http://localhost:* https://unpkg.com https://baremaps.apache.org;
    style-src 'self' 'unsafe-inline' http://127.0.0.1:* http://localhost:* https://unpkg.com https://baremaps.apache.org;
  ">

Can you update the PR? I will then merge it and others will complain if it still has problems. To me things look good 👍

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
100.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@YongGoose
Copy link
Contributor Author

@YongGoose I further investigated the content security policy and I think that the error was comming from a chrome extention that interfered with the policy. It does not show up in incognito mode.

I also tried to better understand why things were not working properly with localhost and 127.0.0.1 and I think it comes from the fact that they are not considered as the same by CSP. So here is a revised meta header works for all cases and is probably better. I also removed the eval-unsafe directive which was unecessary.

  <meta http-equiv="Content-Security-Policy" content="
    default-src 'self' http://127.0.0.1:* http://localhost:* https://unpkg.com https://baremaps.apache.org;
    script-src 'self' http://127.0.0.1:* http://localhost:* https://unpkg.com https://unpkg.com https://baremaps.apache.org 'unsafe-inline';
    worker-src 'self' blob:;
    child-src 'self' blob:;
    img-src 'self' data: blob: http://127.0.0.1:* http://localhost:* https://unpkg.com https://baremaps.apache.org;
    style-src 'self' 'unsafe-inline' http://127.0.0.1:* http://localhost:* https://unpkg.com https://baremaps.apache.org;
  ">

Can you update the PR? I will then merge it and others will complain if it still has problems. To me things look good 👍

Thank you! 🙂

It looks good to me as well!
If any further issues arise, I’ll share them through the mailing list.

Done in 5b5e993 !

@bchapuis bchapuis merged commit 0eb8c1a into apache:main Jan 31, 2025
9 of 10 checks passed
@bchapuis
Copy link
Member

Perfect, thanks a lot for your contribution 🚀

@YongGoose
Copy link
Contributor Author

Perfect, thanks a lot for your contribution 🚀

You're welcome!
I'll do my best to achieve great results with baremaps going forward.

YongGoose added a commit to YongGoose/incubator-baremaps that referenced this pull request Feb 1, 2025
* Update Maplibre to version 5

* Add CSP Directive
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.

Upgrade Maplibre to version 5
2 participants