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

SQLAlchemy patching does not correctly remove password component from URL #131

Closed
WFT opened this issue Feb 15, 2019 · 1 comment
Closed
Labels

Comments

@WFT
Copy link
Contributor

WFT commented Feb 15, 2019

This was noticed when using flask_sqlalchemy, but I believe it affects plain SQLAlchemy as well.

app = Flask(__name__) 
app.config["SQLALCHEMY_DATABASE_URI"] = "postgresql://foo:[email protected]:2834/thing"
db = XRayFlaskSqlAlchemy(app)

Subsegments get recorded with the name postgresql://foo:***@baz.us-east-1.rds.amazonaws.com:2834/thing. Note that the password text is not present in the subsegment name (woohoo!), but it is composed of invalid characters, resulting in an error log message:

Removing Segment/Subsugment Name invalid characters from postgresql://foo:***@baz.us-east-1.rds.amazonaws.com:2834/thing

I believe that the issue lies here:

# Strip password from URL
host_info = u.netloc.rpartition('@')[-1]
parts = u._replace(netloc='{}@{}'.format(u.username, host_info))
safe_url = u.geturl()

The URL is parsed and the password stripped, but the old URL is assigned to safe_url. The method ParseResult._replace returns a new ParseResult but does not modify the initial ParseResult.

Because the fix seems to be so simple, I'll put up a PR before hearing back, but please let me know if there's something I've misunderstood about this.

Configuration:
aws-xray-sdk-python == 2.3.0
Python 3.6

@chanchiem
Copy link
Contributor

Thank you very much for your contribution! I looked at your PR and it looks good to me. I have approved it. Very nice find :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants