firebase / firebase-android-sdk

Firebase Android SDK
https://firebase.google.com
Apache License 2.0
2.25k stars 572 forks source link

157239633: Spaces converted to plus when included in a dynamic link #959

Closed nibarius closed 1 year ago

nibarius commented 4 years ago

Describe your environment

Describe the problem

I've created a deep link with a query parameter that contains A + B. The query parameter is url encoded before being passed to setLink():

FirebaseDynamicLinks.getInstance().createDynamicLink().setLink("https://example.appspot.com/?parameter=A%20%2B%20B")

When I later open this dynamic link the value of the parameter is not what I expect. In the onSuccess method of the OnSuccessListener the value of pendingDynamicLinkData.link is:

https://example.appspot.com/?parameter=A+++B

Both spaces (%20) and pluses (%2b) are converted to + when doing the URL decoding making it impossible to pass any spaces trough a dynamic link.

aguatno commented 4 years ago

Hello @nibarius, thank you for taking time filling this issue! I tried to run a test using Android Quickstart, and the results seem fine upon decoding the value of pendingDynamicLinkData.getLink() "%3DA%20%2B%20B" = "A + B":

KQv3uKA58Dt

In order for us to investigate, could you help me reproduce the issue? That means that I'll need you to create a minimal, complete example that I can run locally.

Thanks :)

nibarius commented 4 years ago

The problem is not with generating the deep links, but with opening them later on. I can reproduce the issue using the Android Quickstart code where I only change the LINK_URL and set up an appropriate url prefix.

dynamic_link

aguatno commented 4 years ago

Thanks @nibarius. I was able to replicate. I'll confirm internally to see if this is an intended behavior or bug (b/144154987), and I'll let you know once I have an update.

aguatno commented 4 years ago

Thanks for waiting @nibarius. I got confirmation from the team that this is intended encoding behaviour where spaces get translated to pluses.

We are using standard Uribuilder to build and store the urls: https://developer.android.com/reference/android/net/Uri.Builder.html#appendQueryParameter(java.lang.String,%20java.lang.String)

In this case, all url will be encoded, not sure if there is a way to decode "A+++B" to "A + B"

aguatno commented 4 years ago

Closing this issue for now since this is an intended behavior. If you have follow questions and want to continue the discussion just leave a comment here and we are happy to re-open this.

nibarius commented 4 years ago

I'm not sure I understand your reasoning. Please take a look at this small example written in Kotlin

fun queryParameter() {
        val key = "key"
        val value = "a + b"
        val builder = Uri.Builder()
        // Encode the query parameter
        val uriString = builder.scheme("https")
                .authority("example.com")
                .appendQueryParameter(key, value)
                .build()
                .toString()
        // uriString is now https://example.com?key=a%20%2B%20b
        // 'a + b' has been encoded to 'a%20%2B%20b'

        // Parse the string again and decode the query parameter
        val uri = Uri.parse(uriString)
        val valueAfter = uri.getQueryParameter(key)
        if (value != valueAfter) {
            throw RuntimeException("value before ('$value') and after ('$valueAfter') storing differs")
        } else {
            // The query parameter which was encoded as 'a%20%2B%20b' has now been
            // decoded to 'a + b' as expected
        }
    }

When I run this the value of the query parameter gets encoded to a%20%2B%20b by the Uribuilder. When I then use the Uri class to decode the query parameter I get the original a + b string back. It should be possible to preserve both spaces and pluses when using the Uri class and the Uri builder.

aguatno commented 4 years ago

Thanks for the code snippet @nibarius. Let me get back to the team regarding your findings, and I'll get back to you once I got an update.

haidaiev-av commented 4 years ago

Hello @aguatno, any updates?

aguatno commented 4 years ago

Sorry for the delay. I'm still waiting for feedback from the team. I'll immediately let you know once I have more information to share.

pedromancheno commented 4 years ago

I am having the same issue in iOS. Any updates?

ashwinraghav commented 4 years ago

Unfortunately this is a standard that cannot really be changed. Spaces may be replaces with either a (+) or a (%20). Are you able to encode the space in your application logic before creating the link?

nibarius commented 4 years ago

I'm not sure I agree with that completely. When doing percent encoding for URIs, like the Firebase deep links, spaces should be encoded as %20. This is explicitly mentioned in RFC 3986 which defines the format of URIs including how data in it should be encoded.

The confusion around this comes from how browsers submits data from forms. Here they use the mime type application/x-www-form-urlencoded (which is not the same as percent encoded URIs).

Wikipedia mentions the following:

The encoding used by default is based on an early version of the general URI percent-encoding rules,[4] with a number of modifications such as newline normalization and replacing spaces with + instead of %20

This section refers to RFC1630, which does mention that space is encoded as plus. It does however also mention that plus signs need to be encoded

Within the query string, the plus sign is reserved as shorthand notation for a space. Therefore, real plus signs must be encoded.

So regardless if the old standard, or the new standard is used, it should always be possible to both encode and decode a string containing both plus and space without loosing any of them.

ashwinraghav commented 4 years ago

That sounds about right. Thanks for pushing this. Are there other characters that encode incorrectly as well ? I'm wondering whether the problem is systemic.

nibarius commented 4 years ago

As far as I know space is the only character where this happens. I think the problem comes from mixing java.net.URLEncoder.encode() with android.net.Uri.decode(). The former "Translates a string into application/x-www-form-urlencoded format using a specific encoding scheme." while the later percent encode all characters that are not unreserved characters, letters or numbers which including space and plus.

Here's an example of what happens when you encode using one scheme and decode using the other. Code:

        val jEnc = java.net.URLEncoder.encode("a + b")
        val jEncjDec = java.net.URLDecoder.decode(jEnc)
        val aEnc = android.net.Uri.encode("a + b")
        val aEncADec = android.net.Uri.decode(aEnc)
        val jEncADec = android.net.Uri.decode(jEnc)
        val aEncJDec = java.net.URLDecoder.decode(aEnc)

        Log.e("encoding", "URLEncoder.encode(): $jEnc, then decoded with URLDecoder.decode(): $jEncjDec")
        Log.e("encoding", "Uri.encode(): $aEnc, then decoded with Uri.decode(): $aEncADec")
        Log.e("encoding", "URLEncoder.encode(): $jEnc, then decoded with Uri.decode(): $jEncADec")
        Log.e("encoding", "Uri.encode(): $aEnc, then decoded with URLDecoder.decode(): $aEncJDec")

Output:

E/encoding: URLEncoder.encode(): a+%2B+b, then decoded with URLDecoder.decode(): a + b
E/encoding: Uri.encode(): a%20%2B%20b, then decoded with Uri.decode(): a + b
E/encoding: URLEncoder.encode(): a+%2B+b, then decoded with Uri.decode(): a+++b
E/encoding: Uri.encode(): a%20%2B%20b, then decoded with URLDecoder.decode(): a + b

The second to last line illustrate exactly the problem we are having here.

JDMathew commented 3 years ago

Is there a fix for this yet?

JDMathew commented 3 years ago

I found that the parameters were not being decoded and so decodeURI(param.replace(/\+/g, '%20')) worked for me. Where param was a parameter string.

piyushsharma21 commented 3 years ago

Am using firebase link shortener api to generate dynamic link, getting similar issue, when I send long link with %20 in it, get short link in response, when opening that short link get the long link with %20 replaced to +. Also I can see similar behavior when creating dynamic links through firebase dashboard.

seemayr commented 1 year ago

This is still happening on iOS. Any updates?

argzdev commented 1 year ago

Hi @seemayr, you can follow up in the firebase-ios-sdk regarding iOS issues.

itachi-05 commented 1 year ago

Had the same issue. Fixed it by using URLDecoder val newDeepLink = URLDecoder.decode(deepLink.toString(), StandardCharsets.UTF_8.name()).toUri()

argzdev commented 1 year ago

Hi folks, we'd like to inform you that the Firebase Dynamic Links service will be shutdown on August 25, 2025. In the meantime, only critical or security issues will be fixed in the SDK.

More at https://firebase.google.com/support/dynamic-links-faq