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

openStream should not be intercepted if Firebase Perf is not initialized #5885

Merged
merged 3 commits into from
May 1, 2024

Conversation

argzdev
Copy link
Contributor

@argzdev argzdev commented Apr 22, 2024

Proposed fix for #5584:

Issue

Crash happens if a URL connection is opened e.g. url.openStream() when TransportManager has not been initialized. If developer wants to programmatically enable Firebase using FirebaseOptions, but have included the Firebase Perf plugin, the app will crash after doing a network call.

Steps to reproduce

  1. Add Firebase perf plugin
  • Module build.gradle
plugins {
    alias(libs.plugins.android.application) apply false
    alias(libs.plugins.jetbrains.kotlin.android) apply false
    id("com.google.gms.google-services") version "4.4.1" apply false
    id("com.google.firebase.firebase-perf") version "1.4.2" apply false
}
  • App build.gradle
plugins {
    alias(libs.plugins.android.application)
    alias(libs.plugins.jetbrains.kotlin.android)
    id("com.google.gms.google-services")
    id("com.google.firebase.firebase-perf")
}
  1. Disable Firebase
        <provider
            android:name="com.google.firebase.provider.FirebaseInitProvider"
            android:authorities="${applicationId}.firebaseinitprovider"
            tools:node="remove"
            />   
  1. Do a network call
val url = URL("https", "www.amazon.com", 443, "")
val inputStream = url.openStream()

Investigation

  1. openStream in FirebasePerfUrlConnection intercepts all urls that opens connections.
  2. openStream requires TransportManager to be initialized
  3. TransportManager is only initialized after FirebasePerformance has been initialized

Solution

If TransportManager is not initialized, we should simply return the URL openStream and not intercept the connection.

Copy link
Contributor

github-actions bot commented Apr 22, 2024

Release note changes

The following release notes were modified. Please ensure they look correct.

Release Notes
firebase-perf
### {{perfmon}} version 21.0.0 {: #performance_v21-0-0}

[fixed] Fixed an `ExceptionInInitializerError` where the `url.openStream()` causes a crash if
FirebasePerf is not yet initialized.

#### {{perfmon}} Kotlin extensions version 21.0.0 {: #performance-ktx_v21-0-0}

The Kotlin extensions library transitively includes the updated
`firebase-performance` library. The Kotlin extensions library has no additional
updates.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 22, 2024

Coverage Report 1

Affected Products

  • firebase-perf

    Overall coverage changed from 70.82% (bb0823f) to 70.81% (0697bf7) by -0.01%.

    FilenameBase (bb0823f)Merge (0697bf7)Diff
    FirebasePerfUrlConnection.java44.26%44.44%+0.18%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/5u9l5yzLjg.html

Copy link
Contributor

github-actions bot commented Apr 22, 2024

Unit Test Results

   110 files   -    840     110 suites   - 840   2m 13s ⏱️ - 32m 47s
   968 tests  - 4 247     968 ✔️  - 4 225  0 💤  - 22  0 ±0 
1 944 runs   - 8 571  1 944 ✔️  - 8 527  0 💤  - 44  0 ±0 

Results for commit 9a59342. ± Comparison against base commit bb0823f.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 22, 2024

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 22, 2024

Startup Time Report 1

Note: Layout is sometimes suboptimal due to limited formatting support on GitHub. Please check this report on GCS.

Notes

Startup Times

  • fire-perf

    DeviceStatisticsDistributions
    oriole-32
    Percentilebb0823f0697bf7DiffSignificant (?)
    p10247 ±21 μs256 ±29 μs+8.20 μs (+3.3%)NO
    p25261 ±22 μs267 ±31 μs+6.87 μs (+2.6%)NO
    p50284 ±29 μs289 ±32 μs+4.30 μs (+1.5%)NO
    p75326 ±40 μs330 ±31 μs+3.61 μs (+1.1%)NO
    p90416 ±74 μs401 ±41 μs-14.9 μs (-3.6%)NO

    20 test runs in comparison
    CommitTest Runs
    bb0823f
    • 2024-04-29_17:31:21.012827_XQDU
    • 2024-04-29_17:31:21.012874_igYs
    • 2024-04-29_17:31:21.012883_gLBo
    • 2024-04-29_17:31:21.012890_qwVh
    • 2024-04-29_17:31:21.012897_bwvi
    • 2024-04-29_17:31:21.012904_RchE
    • 2024-04-29_17:31:21.012912_edvc
    • 2024-04-29_17:31:21.012918_kkcu
    • 2024-04-29_17:31:21.012925_soti
    • 2024-04-29_17:31:21.012932_ieMk
    0697bf7
    • 2024-04-30_18:44:27.647830_aRVY
    • 2024-04-30_18:44:27.647877_UyWS
    • 2024-04-30_18:44:27.647887_TRml
    • 2024-04-30_18:44:27.647894_lQcM
    • 2024-04-30_18:44:27.647901_LuzI
    • 2024-04-30_18:44:27.647909_Kttl
    • 2024-04-30_18:44:27.647915_xnIS
    • 2024-04-30_18:44:27.647922_MnFM
    • 2024-04-30_18:44:27.647929_uJAi
    • 2024-04-30_18:44:27.647936_nSqQ
    redfin-30
    Percentilebb0823f0697bf7DiffSignificant (?)
    p10745 ±84 μs757 ±47 μs+12.0 μs (+1.6%)NO
    p25787 ±94 μs785 ±47 μs-2.57 μs (-0.3%)NO
    p50838 ±100 μs841 ±61 μs+2.96 μs (+0.4%)NO
    p75924 ±112 μs953 ±108 μs+28.6 μs (+3.1%)NO
    p901.02 ±0.1 ms1.10 ±0.2 ms+73.3 μs (+7.2%)NO

    20 test runs in comparison
    CommitTest Runs
    bb0823f
    • 2024-04-29_17:31:21.012827_XQDU
    • 2024-04-29_17:31:21.012874_igYs
    • 2024-04-29_17:31:21.012883_gLBo
    • 2024-04-29_17:31:21.012890_qwVh
    • 2024-04-29_17:31:21.012897_bwvi
    • 2024-04-29_17:31:21.012904_RchE
    • 2024-04-29_17:31:21.012912_edvc
    • 2024-04-29_17:31:21.012918_kkcu
    • 2024-04-29_17:31:21.012925_soti
    • 2024-04-29_17:31:21.012932_ieMk
    0697bf7
    • 2024-04-30_18:44:27.647830_aRVY
    • 2024-04-30_18:44:27.647877_UyWS
    • 2024-04-30_18:44:27.647887_TRml
    • 2024-04-30_18:44:27.647894_lQcM
    • 2024-04-30_18:44:27.647901_LuzI
    • 2024-04-30_18:44:27.647909_Kttl
    • 2024-04-30_18:44:27.647915_xnIS
    • 2024-04-30_18:44:27.647922_MnFM
    • 2024-04-30_18:44:27.647929_uJAi
    • 2024-04-30_18:44:27.647936_nSqQ
  • fire-sessions

    DeviceStatisticsDistributions
    oriole-32
    Percentilebb0823f0697bf7DiffSignificant (?)
    p107.64 ±1 ms6.54 ±0.5 ms-1.10 ms (-14.4%)NO
    p258.06 ±1 ms6.93 ±0.6 ms-1.13 ms (-14.0%)NO
    p508.72 ±1 ms7.61 ±0.8 ms-1.12 ms (-12.8%)NO
    p759.52 ±1 ms8.54 ±1 ms-981 μs (-10.3%)NO
    p9010.7 ±2 ms9.87 ±1 ms-856 μs (-8.0%)NO

    20 test runs in comparison
    CommitTest Runs
    bb0823f
    • 2024-04-29_17:31:21.012827_XQDU
    • 2024-04-29_17:31:21.012874_igYs
    • 2024-04-29_17:31:21.012883_gLBo
    • 2024-04-29_17:31:21.012890_qwVh
    • 2024-04-29_17:31:21.012897_bwvi
    • 2024-04-29_17:31:21.012904_RchE
    • 2024-04-29_17:31:21.012912_edvc
    • 2024-04-29_17:31:21.012918_kkcu
    • 2024-04-29_17:31:21.012925_soti
    • 2024-04-29_17:31:21.012932_ieMk
    0697bf7
    • 2024-04-30_18:44:27.647830_aRVY
    • 2024-04-30_18:44:27.647877_UyWS
    • 2024-04-30_18:44:27.647887_TRml
    • 2024-04-30_18:44:27.647894_lQcM
    • 2024-04-30_18:44:27.647901_LuzI
    • 2024-04-30_18:44:27.647909_Kttl
    • 2024-04-30_18:44:27.647915_xnIS
    • 2024-04-30_18:44:27.647922_MnFM
    • 2024-04-30_18:44:27.647929_uJAi
    • 2024-04-30_18:44:27.647936_nSqQ
    redfin-30
    Percentilebb0823f0697bf7DiffSignificant (?)
    p1012.3 ±2 ms12.4 ±2 ms+67.8 μs (+0.6%)NO
    p2513.1 ±3 ms13.1 ±2 ms-24.1 μs (-0.2%)NO
    p5014.5 ±4 ms14.7 ±4 ms+238 μs (+1.6%)NO
    p7518.1 ±5 ms18.8 ±3 ms+686 μs (+3.8%)NO
    p9021.2 ±5 ms21.2 ±3 ms+43.0 μs (+0.2%)NO

    20 test runs in comparison
    CommitTest Runs
    bb0823f
    • 2024-04-29_17:31:21.012827_XQDU
    • 2024-04-29_17:31:21.012874_igYs
    • 2024-04-29_17:31:21.012883_gLBo
    • 2024-04-29_17:31:21.012890_qwVh
    • 2024-04-29_17:31:21.012897_bwvi
    • 2024-04-29_17:31:21.012904_RchE
    • 2024-04-29_17:31:21.012912_edvc
    • 2024-04-29_17:31:21.012918_kkcu
    • 2024-04-29_17:31:21.012925_soti
    • 2024-04-29_17:31:21.012932_ieMk
    0697bf7
    • 2024-04-30_18:44:27.647830_aRVY
    • 2024-04-30_18:44:27.647877_UyWS
    • 2024-04-30_18:44:27.647887_TRml
    • 2024-04-30_18:44:27.647894_lQcM
    • 2024-04-30_18:44:27.647901_LuzI
    • 2024-04-30_18:44:27.647909_Kttl
    • 2024-04-30_18:44:27.647915_xnIS
    • 2024-04-30_18:44:27.647922_MnFM
    • 2024-04-30_18:44:27.647929_uJAi
    • 2024-04-30_18:44:27.647936_nSqQ
  • timeToInitialDisplay

    DeviceStatisticsDistributions
    oriole-32
    Percentilebb0823f0697bf7DiffSignificant (?)
    p10203 ±3 ms209 ±5 ms+5.67 ms (+2.8%)NO
    p25209 ±3 ms214 ±5 ms+4.92 ms (+2.4%)NO
    p50217 ±4 ms222 ±7 ms+5.66 ms (+2.6%)NO
    p75224 ±4 ms231 ±9 ms+6.78 ms (+3.0%)NO
    p90232 ±4 ms243 ±10 ms+11.1 ms (+4.8%)NO

    20 test runs in comparison
    CommitTest Runs
    bb0823f
    • 2024-04-29_17:31:21.012827_XQDU
    • 2024-04-29_17:31:21.012874_igYs
    • 2024-04-29_17:31:21.012883_gLBo
    • 2024-04-29_17:31:21.012890_qwVh
    • 2024-04-29_17:31:21.012897_bwvi
    • 2024-04-29_17:31:21.012904_RchE
    • 2024-04-29_17:31:21.012912_edvc
    • 2024-04-29_17:31:21.012918_kkcu
    • 2024-04-29_17:31:21.012925_soti
    • 2024-04-29_17:31:21.012932_ieMk
    0697bf7
    • 2024-04-30_18:44:27.647830_aRVY
    • 2024-04-30_18:44:27.647877_UyWS
    • 2024-04-30_18:44:27.647887_TRml
    • 2024-04-30_18:44:27.647894_lQcM
    • 2024-04-30_18:44:27.647901_LuzI
    • 2024-04-30_18:44:27.647909_Kttl
    • 2024-04-30_18:44:27.647915_xnIS
    • 2024-04-30_18:44:27.647922_MnFM
    • 2024-04-30_18:44:27.647929_uJAi
    • 2024-04-30_18:44:27.647936_nSqQ
    redfin-30
    Percentilebb0823f0697bf7DiffSignificant (?)
    p10248 ±4 ms273 ±14 ms+24.6 ms (+9.9%)NO
    p25254 ±4 ms279 ±15 ms+24.9 ms (+9.8%)NO
    p50261 ±6 ms286 ±14 ms+24.9 ms (+9.6%)NO
    p75269 ±8 ms294 ±15 ms+24.8 ms (+9.2%)NO
    p90279 ±12 ms304 ±16 ms+25.3 ms (+9.1%)NO

    20 test runs in comparison
    CommitTest Runs
    bb0823f
    • 2024-04-29_17:31:21.012827_XQDU
    • 2024-04-29_17:31:21.012874_igYs
    • 2024-04-29_17:31:21.012883_gLBo
    • 2024-04-29_17:31:21.012890_qwVh
    • 2024-04-29_17:31:21.012897_bwvi
    • 2024-04-29_17:31:21.012904_RchE
    • 2024-04-29_17:31:21.012912_edvc
    • 2024-04-29_17:31:21.012918_kkcu
    • 2024-04-29_17:31:21.012925_soti
    • 2024-04-29_17:31:21.012932_ieMk
    0697bf7
    • 2024-04-30_18:44:27.647830_aRVY
    • 2024-04-30_18:44:27.647877_UyWS
    • 2024-04-30_18:44:27.647887_TRml
    • 2024-04-30_18:44:27.647894_lQcM
    • 2024-04-30_18:44:27.647901_LuzI
    • 2024-04-30_18:44:27.647909_Kttl
    • 2024-04-30_18:44:27.647915_xnIS
    • 2024-04-30_18:44:27.647922_MnFM
    • 2024-04-30_18:44:27.647929_uJAi
    • 2024-04-30_18:44:27.647936_nSqQ

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/XldVmesTnF/index.html

@argzdev argzdev requested a review from visumickey April 22, 2024 18:54
@argzdev argzdev merged commit 8f2d30e into master May 1, 2024
25 of 27 checks passed
@argzdev argzdev deleted the issue_5584 branch May 1, 2024 14:45
rlazo pushed a commit that referenced this pull request May 2, 2024
…zed (#5885)

Proposed fix for #5584:

### Issue
Crash happens if a URL connection is opened e.g. `url.openStream()` when
`TransportManager` has not been initialized. If developer wants to
programmatically enable Firebase using FirebaseOptions, but have
included the Firebase Perf plugin, the app will crash after doing a
network call.

### Steps to reproduce
1. Add Firebase perf plugin
- Module `build.gradle`
```
plugins {
    alias(libs.plugins.android.application) apply false
    alias(libs.plugins.jetbrains.kotlin.android) apply false
    id("com.google.gms.google-services") version "4.4.1" apply false
    id("com.google.firebase.firebase-perf") version "1.4.2" apply false
}
```
- App `build.gradle`
```
plugins {
    alias(libs.plugins.android.application)
    alias(libs.plugins.jetbrains.kotlin.android)
    id("com.google.gms.google-services")
    id("com.google.firebase.firebase-perf")
}
```
2. Disable Firebase
```xml
        <provider
            android:name="com.google.firebase.provider.FirebaseInitProvider"
            android:authorities="${applicationId}.firebaseinitprovider"
            tools:node="remove"
            />   
```
3. Do a network call
```
val url = URL("https", "www.amazon.com", 443, "")
val inputStream = url.openStream()
```

### Investigation
1. `openStream` in `FirebasePerfUrlConnection` intercepts all urls that
opens connections.
5. `openStream` requires `TransportManager` to be initialized
6. `TransportManager` is only initialized after `FirebasePerformance`
has been initialized

### Solution
If `TransportManager` is not initialized, we should simply return the
URL openStream and not intercept the connection.
@firebase firebase locked and limited conversation to collaborators Jun 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants