meteor-tabular icon indicating copy to clipboard operation
meteor-tabular copied to clipboard

Fix to just project the exact fields are declared on columns and extraFields

Open ricaragao opened this issue 1 year ago • 3 comments

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.

ricaragao avatar May 18 '24 16:05 ricaragao

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.a here and in another ask for config.b it 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 ?

lynchem avatar May 30 '24 10:05 lynchem

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.a here and in another ask for config.b it 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?

ricaragao avatar May 30 '24 14:05 ricaragao

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 avatar May 30 '24 14:05 lynchem

@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 avatar Jul 13 '25 23:07 ricaragao

@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 avatar Jul 14 '25 06:07 jankapunkt

@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

ricaragao avatar Jul 14 '25 13:07 ricaragao