Fix EZP-20601: Permission checking error when using object states

[ghost]

Using object states caused a fatal error in the sql query.
Used the previous query from the 4.6 : http://pubsvn.ez.no/doxygen/4.6.0/html/ezcontentobjecttreenode_8php_source.html#l01633

[m-keil]

Thank you for the patch. This should be definetly merged to trunk-master.

[nfrp]

Hi,

Thanks for the contribution @Arondor !

I wondered whether this fix was related to an existing issue in http://jira.ez.no/browse/EZP (if not, it would make sense to create one, for the sake of tracking, changelog generation).

@m-keil did this apply smoothly and fixed the issue for you?

Cheers guys,

[m-keil]

Hi Nicolas,
this patch was easy to apply and fixed the issue with wrong SQL-Syntax when using object-states.
I'm using ezpublish-legacy trunk version:

Here is the error log entry:

[ Mar 13 2013 09:58:28 ] [10.64.84.33] eZMySQLiDB:
Query error (1054): Unknown column 'ezcontentobject.id' in 'on clause'. Query: SELECT DISTINCT
ezcontentobject.,
ezcontentobject_tree.,
ezcontentclass.serialized_name_list as class_serialized_name_list,
ezcontentclass.identifier as class_identifier,
ezcontentclass.is_container as is_container,
ezcontentobject_name.name as name,
ezcontentobject_name.real_translation
,nmbcounters.count as counter
FROM
ezcontentobject_tree
INNER JOIN ezcontentobject ON (ezcontentobject_tree.contentobject_id = ezcontentobject.id)
INNER JOIN ezcontentclass ON (ezcontentclass.version = 0 AND ezcontentclass.id = ezcontentobject.contentclass_id)
INNER JOIN ezcontentobject_name ON (
ezcontentobject_tree.contentobject_id = ezcontentobject_name.contentobject_id AND
ezcontentobject_tree.contentobject_version = ezcontentobject_name.content_version
), nmbcounters
INNER JOIN ezcobj_state_link ezcobj_state_lnk_0_perm ON (ezcobj_state_lnk_0_perm.contentobject_id = ezcontentobject.id) INNER JOIN ezcobj_state_group ezcobj_state_grp_0_perm ON (ezcobj_state_grp_0_perm.identifier = 'workflow_date') INNER JOIN ezcobj_state ezcobj_state_0_perm ON (ezcobj_state_0_perm.id = ezcobj_state_lnk_0_perm.contentobject_state_id AND ezcobj_state_0_perm.group_id = ezcobj_state_grp_0_perm.id)
WHERE
ezcontentobject_tree.path_string like '/1/2/148/%' and ezcontentobject_tree.depth <= 12 and
ezcontentobject.id = nmbcounters.aid AND nmbcounters.type = 2 AND nmbcounters.identifier = '3d' AND
ezcontentobject_tree.node_id != 148 AND
( ezcontentobject_name.language_id & ezcontentobject.language_mask > 0 AND
( ( ezcontentobject.language_mask - ( ezcontentobject.language_mask & ezcontentobject_name.language_id ) ) & 1 )

• ( ( ( ezcontentobject.language_mask - ( ezcontentobject.language_mask & ezcontentobject_name.language_id ) ) & 2 ) )
  <
  ( ezcontentobject_name.language_id & 1 )
• ( ( ezcontentobject_name.language_id & 2 ) )
  )
  AND ezcontentobject_tree.is_invisible = 0
  AND ((ezcontentobject.contentclass_id in (33, 38, 60, 61, 68, 69, 70, 73) AND ezcontentobject.section_id in (3)) OR (ezcontentobject.section_id in (1) AND ezcobj_state_0_perm.id = 3))
  AND
  ezcontentobject.language_mask & 3 > 0
  ORDER BY counter DESC
  LIMIT 0, 10

Regards
Max

[nfrp]

Thanks Max for the confirmation. Do you happen to know whether there is a related issue in the issue tracker for this?

Thank you,

[m-keil]

I found nothing for this issue.

[jjCavalleri]

(..)

[m-keil]

Hey, I have no permission to open the issue page above. Can you sum up the main points of this ticket?

[jjCavalleri]

@m-keil
You're right. The issue is flagged to be accessible internally only. (..)

[m-keil]

Thank you, @jjCavalleri

[andrerom]

@jjCavalleri The issue you linked to is not related, this is about object states. And given the internal nature of the other issue I have removed your link and info about it.

[nfrp]

Ok, thank you guys for the input.

May I kindly ask you @Arondor and/or @m-keil to create an issue in the issue tracker for this, so we can merge your pull-request in?

Many thanks,

[ghost]

I created this issue https://jira.ez.no/browse/EZP-20601
I hope I did well. Feel free to edit it.

Thanks

[m-keil]

Hey, already 3 weeks ago since the ticket was reported. Can someone please take care of this bug?

Thanks

[andrerom]

@m-keil This is security + sql related in legacy, this is not something we want to break. So review of this will take some time and will probably have to wait until we are done focusing on 5.1.
In the mean time testing (mysql / postgres, with and without extended attribute filter ) by community would be very helpful and speed up the process when last round of review is done.

[jeromegamez]

I can confirm this issue and that the patch fixes it. Would definitely love to see this merged into master!

Thanks, @m-keil and @Arondor for bringing it up!

[jeromegamez]

Any news on this?

[andrerom]

@Arondor Can you rebase this branch on master and push so travis runs the unit tests on it?

[ghost]

I have the impression that it is already on master.
I'm I right?

[andrerom]

@Arondor Whats already on master? Travis tests?: yes

[ghost]

I was talking about the fix.
You asked me to rebase the branch on master. But the commit is already on master.
So I can't do what you're asking.
(sry if I do not understand something, I am not a git expert)

[andrerom]

ah, ok assumed you where doing a topic branch as explained in the "How to contribute to eZ Publish using Git" article.

Ok, in your case it's something like (I assume "upstream" is ezsystems repo and "origin" is your repo):

git fetch upstream
git rebase upstream/master
git push origin master --force

The second command assumes you have already checked out your master locally.

However, doing work on master is not really ideal, you will only be able to do one issue at a time, and you will have to rebase every time you want to start on something new otherwise you will have conflicts with the real master which is on upstream. So if you want you can instead clean it up by doing the following (again assuming you have checked out your own master):

git fetch upstream
git checkout -b 20601_permission_error
git rebase upstream/master
git push origin 20601_permission_error
# To cleanup local master branch
git checkout master
git reset --hard upstream/master
# To also cleanup master on your fork
git push origin master --force
# To also change your local master to instead track ez master, so only for pulling changes
# and creating new topic branches basically
git branch --set-upstream master upstream/master

Then you can close this PR and open a new one based on the new 20601_permission_error branch.

[ghost]

At the time I did the fix I didn't know anything about git. But now I know a thing or two, and avoid working on master is one of them ;)

Instead of pushing on master, I can do what your suggesting on the second part of your post.
I can checkout the master, create the new branch (20601_...), make the changes, rebase and push?

Isn't that a more cleaner way to proceed?
(I wanted to read the "How to contribute" page but the site is down)

[andrerom]

yes there is, here is one of them:

git branch -m master  20601_permission_error
git checkout -b master -t upstream/master
git pull
git push origin master --force
git checkout 20601_permission_error
git rebase master
git push origin 20601_permission_error -u

[ghost]

I have an error :
403 Forbidden while accessing https://github.com/ezsystems/ezpublish-legacy.git/info/refs
while doing :
git push origin 20601_permission_error -u

[andrerom]

It seem origin in your case is ez repo and not your own, adapt the examples to fit your setup ;)

[ghost]

I wasn't aware of the way to configure "upstream" and "origin".
So I set "origin" to be my repo and "upstream" to be eZ repo, and I did :

git branch -m master  20601_permission_error
git checkout -b master -t upstream/master
git pull
git push origin master --force
git checkout 20601_permission_error
git rebase master
git push origin 20601_permission_error -u

I think it went ok.
So know I have to open a new pull request and merge the 20601_... branch?

[andrerom]

Yes (open pr but not merge), but I did it for you, closing this one and moving discussion to #745

[ghost]

Thank for your help.
First time I used pull-request and fork.

I understand the whole thing much better now :)