Rob
(Rob)
24 August 2022 09:11
1
Hi all,
I just found a bug in the specklepy StreamWrapper, see this line:
specklepy/wrapper.py at main · specklesystems/specklepy · GitHub
The issue is in the check: if self.host in a.serverInfo.url
. Namely, we have our own company Speckle server (rhdhv.speckle.xyz), but I was currently testing with the public server (speckle.xyz). Then of course when performing this check, it will evaluate to True for "speckle.xyz" in "rhdhv.speckle.xyz"
, meaning that the wrong account (of rhdhv.speckle.xyz) can possibly be assigned to a StreamWrapper with a stream_url of speckle.xyz. This of course gives issues when actually calling the API, returning a GraphQLException.
I now fixed it like below. Would be nice to have a generic fix for it, as I can see this issue happening for more users that will start to use their own server at some point.
3 Likes
gokermu
(Mucahit Bilal GOKER)
24 August 2022 11:59
2
Hi @Rob ,
Thanks for bringing this up. I’ll ping our PyQueen🐍 @izzylys on this.
1 Like
gokermu
(Mucahit Bilal GOKER)
26 September 2022 11:37
3
Hey @Rob ,
I created an issue on Github. Thanks for reporting it .
opened 11:35AM - 26 Sep 22 UTC
bug
## Description
Mentioned by [Rob ](https://speckle.community/u/rob)on [this pos… t](https://speckle.community/t/bug-in-specklepy-streamwrapper/3442):
> I just found a bug in the specklepy StreamWrapper, see this line:
> [specklepy/wrapper.py at main · specklesystems/specklepy · GitHub 1](https://github.com/specklesystems/specklepy/blob/main/specklepy/api/wrapper.py#L113)
>
> The issue is in the check: if self.host in a.serverInfo.url. Namely, we have our own company Speckle server (rhdhv.speckle.xyz), but I was currently testing with the public server (speckle.xyz). Then of course when performing this check, it will evaluate to True for "speckle.xyz" in "rhdhv.speckle.xyz", meaning that the wrong account (of rhdhv.speckle.xyz) can possibly be assigned to a StreamWrapper with a stream_url of speckle.xyz. This of course gives issues when actually calling the API, returning a GraphQLException.
## Related Line
https://github.com/specklesystems/specklepy/blob/d0eb364b3ef45c30182be32448ce5138987b1984/specklepy/api/wrapper.py#L113
## Workaround
> I now fixed it like below. Would be nice to have a generic fix for it, as I can see this issue happening for more users that will start to use their own server at some point.
## Screenshots
![](https://speckle.community/uploads/default/optimized/2X/c/c7f25a5dcc99e7ae6c1ad9e12770b9f8d882bf65_2_690x183.png)
1 Like
gergo
(Gergő Jedlicska)
26 September 2022 18:19
4
Hey @Rob
sorry for acting late on this, but now we have a PR ready with a fix. It should make its way to a new specklepy
release soon.
specklesystems:main
← specklesystems:gergo/urlCompare
opened 06:16PM - 26 Sep 22 UTC
- Fix warnings about None type units being set on Base objects Add proper units … enum implementation
- update tests
- remove checking for the fetched users email
- test for python311
- py311 is not yet supported by circleci base images
- reformat stuff, fix backwards compatible typing
- remove unused literal
- remove literal import
- fix stream wrapper matching on accounts with subdomain url-s
<!---
Provide a short summary in the Title above. Examples of good PR titles:
* "Feature: adds metrics to component"
* "Fix: resolves duplication in comment thread"
* "Update: apollo v2.34.0"
-->
## Description & motivation
<!---
Describe your changes, and why you're making them. What benefit will this have to others?
Is this linked to an open Github issue, a thread in Speckle community,
or another pull request? Link it here.
If it is related to a Github issue, and resolves it, please link to the issue number, e.g.:
Fixes #85, Fixes #22, Fixes username/repo#123
Connects #123
-->
## Changes:
<!---
- Item 1
- Item 2
-->
## To-do before merge:
<!---
(Optional -- remove this section if not needed)
Include any notes about things that need to happen before this PR is merged, e.g.:
- [ ] Change the base branch
- [ ] Ensure PR #56 is merged
-->
## Screenshots:
<!---
Include a screenshot the before and after. This can be a screenshot of a plugin, web frontend, or output in a terminal.
-->
## Validation of changes:
<!---
Describe what tests have been added or amended, and why these demonstrate it works and will prevent this feature being accidentally broken by future changes.
-->
## Checklist:
<!---
This checklist is mostly useful as a reminder of small things that can easily be
forgotten – it is meant as a helpful tool rather than hoops to jump through.
Put an `x` between the square brackets, e.g. [x], for all the items that apply,
make notes next to any that haven't been addressed, and remove any items that are not relevant to this PR.
-->
- [x] My pull request follows the guidelines in the [Contributing guide](https://github.com/specklesystems/speckle-server/blob/main/CONTRIBUTING.md)?
- [x] My pull request does not duplicate any other open [Pull Requests](../../pulls) for the same update/change?
- [x] My commits are related to the pull request and do not amend unrelated code or documentation.
- [x] My code follows a similar style to existing code.
- [ ] I have added appropriate tests.
- [x] I have updated or added relevant documentation.
## References
<!---
(Optional -- remove this section if not needed )
Include **important** links regarding the implementation of this PR.
This usually includes a RFC or an aggregation of issues and/or individual conversations
that helped put this solution together. This helps ensure we retain and share knowledge
regarding the implementation, and may help others understand motivation and design decisions etc..
-->
1 Like
Rob
(Rob)
27 September 2022 07:05
5
Great, thanks for the update!
Let me know when it’s there so we can have a test
1 Like