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

Fix matching forbidden types in set component command #2872

Draft
wants to merge 2 commits into
base: legacy/v5
Choose a base branch
from

Conversation

traksag
Copy link
Contributor

@traksag traksag commented Jul 8, 2020

Overview

Gets rid of problems such as 'grass_block' matching 'grass' if 'grass' is a forbidden type.

Fixes [no issue report] (see below)

Description

Create a clean install of PlotSquared, claim a plot, remove all of your PlotSquared permissions and give yourself plots.set.*. Now run /p set floor grass_block. You'll get the error:

You are not allowed to generate a component containing the block 'grass'.

This pull request aims to resolve this.

Checklist

  • I included all information required in the sections above
  • I tested my changes and approved their functionality
  • I ensured my changes do not break other parts of the code
  • I read and followed the contribution guidelines

@Citymonstret
Copy link
Member

The reason we switched to contains was because we don't process stuff such as block states, so that "grass[]" wouldn't be a bypass. The only real way to get around this would be by using regex to extract the actual block type from the pattern. We opted for the current solution as it would indeed cover all cases, but it could probably have been handled better.

Gets rid of problems such as 'grass_block' matching 'grass' if
'grass' is a forbidden type.
@traksag
Copy link
Contributor Author

traksag commented Jul 8, 2020

Hmm, so the set command actually supports all WE patterns, which means something like /p set floor hand works if grass isn't in the disallowed-blocks section of the WorldEdit config and you have grass in your hand.

@SirYwell
Copy link
Member

SirYwell commented Jul 8, 2020

We need to handle that differently. As most patterns are either RandomBlockPattern, RandomBlockStatePattern or some instance of BlockStateHolder, we could probably check those instead. That way, we might have issues with custom patterns provided by third party plugins, but as you stated, we have issues using the current way too.

Instead of trying to parse patterns ourselves to figure out if invalid
blocks are used, let WE handle it by modifying the disallowed blocks
list. This ensures things like '/p set floor hand' only work for
allowed blocks.

Currently it seems some of the WE pattern parsers don't check against
the disallowed blocks list. E.g. BlockCategoryPatternParser doesn't,
so '/p set floor ##buttons' works even if all buttons are disallowed.
@Citymonstret
Copy link
Member

Well that'd also affect #clipboard. The solution to that would be to parse the pattern without supplying a player, but then we lose all context based patterns.

@traksag
Copy link
Contributor Author

traksag commented Jul 8, 2020

The DefaultBlockParser (see here) uses WorldEdit's disallowed blocks config option if the player doesn't have worldedit.anyblock to filter out any bad blocks. I added a proof of concept commit that exploits this. Sadly not all pattern parsers use the disallowed blocks config option (is that a WE bug?).

@traksag
Copy link
Contributor Author

traksag commented Jul 8, 2020

Maybe it would indeed be best to just check the parsed pattern explicitly. Although currently WE doesn't allow you to check which blocks are used in e.g. RandomPattern because all fields are private and there are no public getters...

@Citymonstret
Copy link
Member

I had that idea too, but there's no good way to iterate over all blocks involved in a pattern. The only way I could think of, which sounds awful, would be to manually go through every block that would be updated and check what the pattern returns. Not a very pleasant thought, lol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants