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

Postgres: Safely escape strings in nested objects #5855

Merged
merged 3 commits into from Jul 29, 2019

Conversation

@dplewis
Copy link
Member

commented Jul 26, 2019

Superced: #5336

Removed existing error message and prevent query from breaking using pg-promise built in formatting engine

dplewis added 2 commits Jul 26, 2019
@codecov

This comment has been minimized.

Copy link

commented Jul 27, 2019

Codecov Report

Merging #5855 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5855      +/-   ##
==========================================
- Coverage   93.69%   93.67%   -0.03%     
==========================================
  Files         150      150              
  Lines       10616    10613       -3     
==========================================
- Hits         9947     9942       -5     
- Misses        669      671       +2
Impacted Files Coverage Δ
...dapters/Storage/Postgres/PostgresStorageAdapter.js 96.97% <100%> (+0.07%) ⬆️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 91.84% <0%> (-0.72%) ⬇️
src/RestWrite.js 93.56% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1903f59...79596c4. Read the comment docs.

@vitaly-t

This comment has been minimized.

Copy link
Contributor

commented Jul 28, 2019

Hypothetically, the first half of the PostgreSQL adapter could be replaced with much nicer code, because you ignore that pg-promise supports nested properties natively, and the power of Custom Type Formatting. I would have done it myself, but I'm no expert in parse-server to do such a rewrite.

@dplewis

This comment has been minimized.

Copy link
Member Author

commented Jul 28, 2019

I’ve been meaning to do a rewrite at some point. The PG Adapter is ~2500 lines.

I didn’t know it supported nested natively and custom types

If you could do an initial PR, I can help out.

@vitaly-t

This comment has been minimized.

Copy link
Contributor

commented Jul 28, 2019

I'm afraid I'm not the right person to rewrite it, I know almost nothing about the code actual purpose, as I do not even use parse-server myself. But if you have any questions, I will answer ;) And besides, you'd want to rewrite the whole thing, not a small part of it.

@davimacedo
Copy link
Member

left a comment

It looks good to me. LGTM!

@dplewis dplewis merged commit 95208e9 into master Jul 29, 2019

2 checks passed

Danger All good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dplewis dplewis deleted the pg-nested-string branch Jul 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.