[PM.24380] fix: Correct and redact flight recorder hostname on logs#6633
Open
[PM.24380] fix: Correct and redact flight recorder hostname on logs#6633
Conversation
Replacing api url with selfhosted one on toFailure at NetworkResultCall
github-actions
bot
added
app:password-manager
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6633 +/- ##
==========================================
+ Coverage 85.91% 86.41% +0.49%
==========================================
Files 801 789 -12
Lines 57098 56791 -307
Branches 8305 8310 +5
==========================================
+ Hits 49056 49074 +18
+ Misses 5160 4834 -326
- Partials 2882 2883 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
|
Great job! No new security vulnerabilities introduced in this pull request |
aj-rosado
changed the title
vvolkgang
reviewed
Mar 10, 2026
| val url = if (actualHost != null) { | ||
| "${originalUrl.protocol}://$actualHost${originalUrl.path}" | ||
| } else { | ||
| "${originalUrl.protocol}://${originalUrl.authority}${originalUrl.path}" |
Member
There was a problem hiding this comment.
Do we need to rebuild here, can't we return originalUrl?
SaintPatrck
reviewed
Mar 10, 2026
Contributor
SaintPatrck
left a comment
SaintPatrck
left a comment
There was a problem hiding this comment.
❓ I recently added logging of hostname to CookieIntercepter. Do those need to be checked/redacted also?
| throwable?.let { | ||
| bw.append(" – ") | ||
| bw.append(it.getStackTraceString()) | ||
| bw.append(it.getStackTraceString().redactUrls()) // Also redact stack traces |
|
|
||
| // Check if this is a hardcoded default URL that will be replaced by BaseUrlInterceptor | ||
| // Match against the defaults from RetrofitsImpl.kt line 111 and EnvironmentUrlDataJson | ||
| val actualHost = if (baseUrlsProvider != null) { |
| // Match against the defaults from RetrofitsImpl.kt line 111 and EnvironmentUrlDataJson | ||
| val actualHost = if (baseUrlsProvider != null) { | ||
| when (originalUrl.host) { | ||
| "api.bitwarden.com" -> baseUrlsProvider.getBaseApiUrl().toHttpUrlOrNull()?.host |
Collaborator
There was a problem hiding this comment.
We can avoid all this complexity with an Interceptor that handles the error logging.
class NetworkErrorLogInterceptor : Interceptor {
override fun intercept(chain: Interceptor.Chain): Response {
return chain
.proceed(chain.request())
.also {
if (!it.isSuccessful) {
val url = it.request.url.toUrl().run { "$protocol://$authority$path" }
Timber.e("Network Error: $url")
}
}
}
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-24380
📔 Objective
This aims two address different scenarios.
BaseUrlInterceptorhaving updated the url. So now we are checking on theNetworkResultCallif we should update the BaseUrl using the existingBaseUrlsProvider