jsonresume / theme-manager

Theme server for JSON Resume
MIT License
166 stars 31 forks source link

Minor cleanup and bug fixes #11

Open remydagostino opened 10 years ago

remydagostino commented 10 years ago

There are some pretty big security holes in this lib. I would have liked to have fixed them up but I found the code was deeply nested and hard to work with. I've made some minor fixes to the structure - just extracting methods and removing some dependencies on shared scopes. I'm not perfectly happy with the result but I do think it moves us somewhat toward a code base that can be more easily worked on.

I also found a couple of bugs while I was working, which I believe I have fixed.

Other than the clean-up and those bug fixes there should not be any material changes to the behaviour of the theme manager. Moving forward I think we should talk about a more major restructuring of the code as the current implementation is very fragile and insecure.

This is my first open source contribution. Please be kind if I have made any faux pas.

thomasdavis commented 10 years ago

Yeah, those security issues are pretty high priority now that we are getting some traction.

Will look at the changes later tonight, sounds pretty good though.

Still overseas?

remydagostino commented 10 years ago

Thanks. I'm back in Brisbane for at least another week, maybe two.

I'm not sure what the best way to handle the security issues would be. At this point though I think it might just be best to get them documented and clearly flag the unsafe functions so that some linux security wizard can come along and fix them. :)

PeterDaveHello commented 9 years ago

any updates?