mapbox / mapbox/mapbox-gl-js

Attribution should not use innerHTML=

Open
#7,915 5 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

needs discussion :speech_balloon:
Dominant language
TypeScript
Stars
12.4k
Forks
2.4k
PR merge metrics
No merged PRs in 30d

Description

## Motivation

While working on #7914 to add `rel="noopener"` to cross-domain links, I noticed that attribution control links are [directly injected using `innerHTML=`](https://github.com/mapbox/mapbox-gl-js/blob/6d529c74ac3524749bfb50a4e6f5c991f4d058c7/src/ui/control/attribution_control.js#L160). For me this raises a couple of questions:

1. How can I get `rel="noopener"` added to these links? As it stands, this would have to change on the data source side. Is HTML really the best way to communicate attribution?
2. Using `innerHTML=` is a potential security issue. While mapbox is obviously a trusted 3rd party, there are certain use cases where having this opportunity for remote execution would be very undesirable. This is especially important if using third party sources – until I read the source code just now, I thought I was simply consuming their data, and didn't realize that I was essentially giving them full access to the contents of my page.

## Design Alternatives

1. Parse the HTML on the client side, and reconstruct DOM links accordingly. It looks like [mapbox-gl-native already does this](https://github.com/mapbox/mapbox-gl-native/blob/d5659aa6647f1fc77159567bd22029a2dc9cd7a3/platform/darwin/src/MGLAttributionInfo.mm#L20) to make its attribution links. This seems a bit silly to do, but is probably the best way of fixing this while being backwards-compatible.
2. The most straightforward fix is to change the source attribution format to separately describe the titles and URLs of each source. However, this would require a change to the spec and break all sorts of existing implementations.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Attribution links are built in src/ui/control/attribution_control.js; start there and compare the parsing approach in the linked mapbox-gl-native MGLAttributionInfo.mm. Resolve whether the backwards-compatible alternative or a source-format change is intended; done means attribution remains functional without direct innerHTML injection and the link behavior is covered by tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.