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

Store rendered Markdown/Markup #638

Open
sizzlemctwizzle opened this issue Jun 5, 2015 · 11 comments
Open

Store rendered Markdown/Markup #638

sizzlemctwizzle opened this issue Jun 5, 2015 · 11 comments
Labels
enhancement Something we do have implemented already but needs improvement upon to the best of knowledge. security Usually relates to something critical.

Comments

@sizzlemctwizzle
Copy link
Member

var mongoose = require('mongoose');
var Schema = mongoose.Schema;

var renderedContentSchema = new Schema({
  content: String,
  model: String,
  _contentId: Schema.Types.ObjectId
});

var RenderedContent  = mongoose.model('RenderedContent', renderedContentSchema);

exports.RenderedContent = RenderedContent;

This is where output from renderMd can be stored, so that we only render markdown when it changes.

@sizzlemctwizzle sizzlemctwizzle added the enhancement Something we do have implemented already but needs improvement upon to the best of knowledge. label Jun 5, 2015
@sizzlemctwizzle sizzlemctwizzle changed the title Store rendered HTML Store rendered Markdown/Markup Jun 5, 2015
@Martii
Copy link
Member

Martii commented Jun 5, 2015

This might be considered a bad idea if our sanitizer discovers a security hole and we have to redo the entire database. There is already precedence for this occasion.

@sizzlemctwizzle
Copy link
Member Author

I'll build in a routine to re-render all stored content. I'll even put a
button for it on the admin dashboard.
On Jun 5, 2015 9:51 AM, "Marti Martz" notifications@github.com wrote:

This might be considered a bad idea if our sanitizer discovers a security
hole and we have to redo the entire database. There is already precedence
for this occasion.


Reply to this email directly or view it on GitHub
#638 (comment)
.

@Martii
Copy link
Member

Martii commented Jun 5, 2015

Perhaps a flag check based off last updated compared to last rerendering date could work too to let the users do it on revisitation of those affected pages could work too... but we don't currently store that information in the db.

@Martii Martii added the security Usually relates to something critical. label Jun 5, 2015
@sizzlemctwizzle
Copy link
Member Author

Then we'd still have a security hole as long as the content hasn't
expired. With my process we could fix everything immediately. Plus less
re-rendering.
On Jun 5, 2015 10:03 AM, "Marti Martz" notifications@github.com wrote:

Perhaps a flag check based off last updated compared to last rerendering
date could work too to let the users do it on revisitation of those
affected pages could work too... but we don't currently store that
information in the db.


Reply to this email directly or view it on GitHub
#638 (comment)
.

@Martii
Copy link
Member

Martii commented Jun 5, 2015

Not quite... on access it would check lastupdate versus the lastforced and any visitor would trigger the rendering and rerender it if needed... thus the (potentially) infected content never gets displayed to anyone. Your methodology is less scalable and will take a very long time when the db is much larger... USO had downtime because of this e.g. hours.

@Martii
Copy link
Member

Martii commented Jun 5, 2015

Related to #81 and #601 as well... one big db redo is going to be traffic intensive.

@sizzlemctwizzle
Copy link
Member Author

I see what you mean and I agree. This let's us re-render when we need to
rather than all at once.
On Jun 5, 2015 10:12 AM, "Marti Martz" notifications@github.com wrote:

Not quite... on access it would check lastupdate versus the lastforced and
any visitor would trigger the rendering and rerender it... thus the
infected content never gets displayed to anyone. Your methodology is less
scalable and will take a very long time when the db is much larger... USO
had downtime because of this e.g. hours.


Reply to this email directly or view it on GitHub
#638 (comment)
.

@joeytwiddle
Copy link
Contributor

If this collection is just a cache, you could wipe the entire collection if a rendering issue is discovered. No need to store/compare dates.

I agree re-rendering and caching on demand sounds more scalable!

@sizzlemctwizzle
Copy link
Member Author

@joeytwiddle might be right. I don't think it takes long destroy an entire collection. Once the collection is gone, all content will be forced re-render on demand. Of course our inevitable need to use sharding may make this slower. I really don't know.

@Martii
Copy link
Member

Martii commented Jun 10, 2015

@sizzlemctwizzle
That's a null check to see if it's even been rendered e.g. date check... lastchecked vs lastforced is a general analogy which can easily be compared to before time and the last time. e.g. null vs exists. "Wiping" will still take longer especially when considering all comments/issues/discussions are rendered as well and not just script homepages... our DB isn't that large at the moment but give it a few more years and the performance could be potentially adversely affected.

@Martii
Copy link
Member

Martii commented Jun 10, 2015

Another miscellaneous note as well... DB compaction... eventually wiping could potentially fragment the database even worse than an overwrite... we haven't touched on that yet since the change at d64f754

Since OUJS is now on SSD's it might be prudent to check the warranty status with the host provider and how often they perform backups of our VPS, replacements, etc. and if they do any compression anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something we do have implemented already but needs improvement upon to the best of knowledge. security Usually relates to something critical.
Development

No branches or pull requests

3 participants