Skip to content

fix requests with compression (replay)#8

Open
xliee wants to merge 3 commits intogbip:mainfrom
xliee:main
Open

fix requests with compression (replay)#8
xliee wants to merge 3 commits intogbip:mainfrom
xliee:main

Conversation

@xliee
Copy link

@xliee xliee commented Jul 25, 2023

No description provided.

Copy link
Author

@xliee xliee left a comment

Choose a reason for hiding this comment

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

ok

Copy link
Owner

@gbip gbip left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, my github notifications were muted ...

I thinks this is a useful feature, however I can not merge this PR as is.

.and_then(|v| v.to_str().ok())
.and_then(|s| s.split(',').next())
.and_then(|s| s.trim().parse::<std::net::IpAddr>().ok())
.unwrap_or("0.0.0.0".parse::<std::net::IpAddr>().unwrap())
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we could use the request source IP there ?

It could be useful if you were to investigate a broken chain of reverse-proxy where one of them does not propagate 'X-Forwared-For' correctly.

// Err(AError::new(BodyError::InvalidNumberOfLines))
// }

// Parse line by line until we find the dsn (Max 10 lines)
Copy link
Owner

Choose a reason for hiding this comment

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

Can you update this comment to match the range line 156.

}
} else {
Err(AError::new(BodyError::MissingDsnKeyInHeader))
}
Copy link
Owner

Choose a reason for hiding this comment

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

Don't you think that we can keep those error types as a way to provide more insights when debugging sentry tunnels ?

pub raw_body: Vec<u8>,
pub dsn: Dsn,
pub is_safe: bool,
pub user_ip: String,
Copy link
Owner

Choose a reason for hiding this comment

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

How about making this an Enum representing the various IP that we can have :

  • an ip from x-forwarded-for
  • the client IP

dsn,
raw_body: fullBody,
is_safe,
user_ip: String::from("unknown"),
Copy link
Owner

Choose a reason for hiding this comment

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

Is "unknown" some kind of special value that is handled later ?

dasJ added a commit to dasJ/sentry_tunnel that referenced this pull request Apr 10, 2024
Extracted that part from gbip#8
@mat813
Copy link

mat813 commented Jun 20, 2024

Ah, I was wondering why I got errors like invalid utf-8 sequence of 1 bytes from index 1172 with session replay. I'll disable compression for the time being.

Conni2461 pushed a commit to helsinki-systems/sentry_tunnel that referenced this pull request Aug 23, 2024
Conni2461 added a commit to helsinki-systems/sentry_tunnel that referenced this pull request Aug 23, 2024
Conni2461 added a commit to helsinki-systems/sentry_tunnel that referenced this pull request Aug 23, 2024
@bustEXZ
Copy link

bustEXZ commented May 28, 2025

any updates?

@gbip
Copy link
Owner

gbip commented May 28, 2025

any updates?

I am not planning on working on this tool in the future month.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants