BridgesUNCC / bridges-java

JAVA Client library for Bridges
http://bridgesuncc.github.io
GNU General Public License v2.0
4 stars 12 forks source link

using JSONValue escape to clean strings #121

Closed squeetus closed 5 years ago

squeetus commented 6 years ago

Rather than relying on custom code to validate JSON strings for Title, Description, and Element Label attributes, these commits use JSONValue.escape("..."); to clean user strings submitted with an assignment's JSON.

esaule commented 5 years ago

This seems like a better approach. I wonder if we should look back at all the JSON consturction and call JSONValue.escape where appropriate. I may look into that tomorrow.

krs-world commented 5 years ago

Yeah..

Speaking of which, shoujld we be looking at  more actively using rapidjson to build our strings like its being done in Java. Looks like a lot of work. Not sure if we have time for that by Spring.

    -- krs

On 12/26/18 6:53 PM, Erik Saule wrote:

This seems like a better approach. I wonder if we should look back at all the JSON consturction and call JSONValue.escape where appropriate. I may look into that tomorrow.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/krs-world/bridges/pull/121#issuecomment-450042739, or mute the thread https://github.com/notifications/unsubscribe-auth/AFZSOJBX90880j0vQmKw6rNT2pl8gaV-ks5u9AvngaJpZM4WEev-.

-- Kalpathi Subramanian Ph: 704 687 8579 Associate Professor Email: krs@uncc.edu Dept of Computer Science Web:http://webpages.uncc.edu/krs The University of North Carolina Charlotte, NC 28202-0001

esaule commented 5 years ago

I am confused.In the C++ version, we are converting all user input strings to a JSON string using rapidjson. We have been doing that for about 6 month. But the java version does not AFAIK. Can you clarify what you meant?

-- Erik Saule Assistant Professor at UNC Charlotte Tel: +1 (704) 687-8580

Discover new academic papers with theAdvisor http://theadvisor.osu.edu/

esaule commented 5 years ago

Actually, this commit already made it through the main branch. So I am closing this pull request.