meteor-tabular
meteor-tabular copied to clipboard
Fix to just project the exact fields are declared on columns and extraFields
Tabular has an amazing feature that is take care of the publications to just publish the fields are defined in columns and extraFields.
But there was an issue when you have a document/object with sub-documents, because the code
fields[cleanFieldName(data)] = 1;
execute cleanFieldName function to set what fields should be projected. When you have subdocuments and per example you set a column data with parent.child, cleanFieldName will get just the parent, then if you have a big document, all that fields will be published. This can be a security issue too.
I changed that line to doesn't clean the the field name, than unnecessary fields will be published.
fields[data] = 1;
This simple change will save a lot of traffic but can have a little negative impact on systems that are using another subdocuments that are not explicit published. Anyway, I guess this is a important change.
Hey @ricaragao , sorry about taking so long to reply. I think this is a better way of doing it. I only have two concerns:
- if someone is using this and inadvertently relying on getting more fields
- if they have another publication active on the collection I think merge-box can't handle subfields. So if you ask for
config.ahere and in another ask forconfig.bit will overwrite it.
Maybe we can make it opt-out ? So a config option saying cleanFieldName which defaults to true and you can set it in the tabular to turn it off ?
Hey @ricaragao , sorry about taking so long to reply. I think this is a better way of doing it. I only have two concerns:
- if someone is using this and inadvertently relying on getting more fields
- if they have another publication active on the collection I think merge-box can't handle subfields. So if you ask for
config.ahere and in another ask forconfig.bit will overwrite it.Maybe we can make it opt-out ? So a config option saying cleanFieldName which defaults to true and you can set it in the tabular to turn it off ?
Hi @lynchem , yes, can be a good idea use a opt-out. Do you think it can be on main configuration object?
Hi @lynchem , yes, can be a good idea use a opt-out. Do you think it can be on main configuration object?
Yup, sounds good 👍🏻
@lynchem and @jankapunkt , I created a new PR following the @lynchem suggestion and point it to the migration version branch.
I'm closing this PR.
@ricaragao was it outdated or closed due to no activity? I currently don't actively use tabular so I had no reference project for testing.
@jankapunkt , not exactly. I just did the same modification in another branch with a new option where the user can choose if they want this behavior or not. The default will be as before. This new branch I did a PR with target to the migration branch. You can see that it is a PR with more information: https://github.com/Meteor-Community-Packages/meteor-tabular/pull/466