Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

mapbox: URL containing query string causes infinite loop #5554

Merged
merged 4 commits into from
Jul 4, 2016
Merged

mapbox: URL containing query string causes infinite loop #5554

merged 4 commits into from
Jul 4, 2016

Conversation

jingsam
Copy link
Contributor

@jingsam jingsam commented Jul 4, 2016

If getMapboxURLPathname parse a url with querystring, such as mapbox://p/a/t/h?query=aa, an infinite loop happens and crash your computer. This is a serious bug which will affect all platforms. You should fix it ASAP!

@1ec5 1ec5 added the crash label Jul 4, 2016
@1ec5 1ec5 changed the title [core]Fix a serious bug in getMapboxURLPathname mapbox: URL containing query string causes infinite loop Jul 4, 2016
@1ec5 1ec5 added the Core The cross-platform C++ core, aka mbgl label Jul 4, 2016
mbgl::util::mapbox::normalizeSourceURL("mapbox://user.map", "key"));
EXPECT_EQ(
"http://path",
mbgl::util::mapbox::normalizeSourceURL("http://path", "key");
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is missing a closing parenthesis.

@jingsam
Copy link
Contributor Author

jingsam commented Jul 4, 2016

Should be OK now. @1ec5

@1ec5 1ec5 merged commit e82ecc6 into mapbox:master Jul 4, 2016
@1ec5
Copy link
Contributor

1ec5 commented Jul 4, 2016

Verified that the tests build and pass locally. Thanks for the fix!

@jingsam
Copy link
Contributor Author

jingsam commented Jul 4, 2016

I checked that the implement in mapbox.cpp would have some potential bugs when URL contains query string. So I plan to refactor this file to fully compatible with query string and hash tags

@jingsam
Copy link
Contributor Author

jingsam commented Jul 4, 2016

Note that my development environment is an Ubuntu server VM on Windows 10. Mapbox-gl-native can not run on my VM, it is a bit of un-convenient to test my modification

@1ec5
Copy link
Contributor

1ec5 commented Jul 4, 2016

Unfortunately, I’m not very familiar with our Linux build support, but here are instructions in case you haven’t seen them. Otherwise, another option might be to build the Android SDK on Linux, although that’s a bit further removed from the code you’re working with.

@1ec5 1ec5 added this to the ios-v3.3.1 milestone Jul 19, 2016
1ec5 added a commit that referenced this pull request Jul 19, 2016
Updated changelogs for #5723, #5554.
@1ec5
Copy link
Contributor

1ec5 commented Jul 19, 2016

Updated the iOS and macOS changelogs in a53dfcd.

1ec5 pushed a commit that referenced this pull request Jul 19, 2016
* correct all EXPECT_EQ(actual, expected) to EXPECT_EQ(expected, actual)

* fix getMapboxURLPathname() of URL with querystring

* add test for normalizeSourceURL of non-mapbox protocal

* Update mapbox.cpp

Cherry-picked from e82ecc6.
1ec5 added a commit that referenced this pull request Jul 19, 2016
Updated changelogs for #5723, #5554.

Cherry-picked from a53dfcd.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl crash
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants