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

Validate XPath query before use it with the new bblfsh api #635

Closed
ajnavarro opened this issue Nov 28, 2018 · 7 comments
Closed

Validate XPath query before use it with the new bblfsh api #635

ajnavarro opened this issue Nov 28, 2018 · 7 comments
Labels
blocked Some other issue is blocking this enhancement New feature or request

Comments

@ajnavarro
Copy link
Contributor

More info here: https://github.com/bblfsh/sdk/blob/4ba719e80ab52f2d4da5ac8a6f4675687c6fe952/uast/query/xpath/xpath.go#L22

@ajnavarro ajnavarro added the enhancement New feature or request label Nov 28, 2018
@kuba-- kuba-- self-assigned this Nov 28, 2018
@kuba--
Copy link
Contributor

kuba-- commented Nov 28, 2018

@ajnavarro, @bzz
I think we already uses the new stuff!

In function func (u *uastFunc) getUAST we call nodeArray, err = applyXpath(node, xpath) (https://github.com/src-d/gitbase/blob/master/internal/function/uast.go#L250)

applyXpath calls it, err := tools.Filter(n, query) (where tools.Filter is gopkg.in/bblfsh/client-go.v3/tools/context.go#L116)
It creates a new context with xpath: xpath.New(), and calls xpath.Execute:

if query == "" {
		query = "//*"
}
return c.xpath.Execute(c.root, query)

where c.xpath.Execute is exactly the function:
https://github.com/src-d/gitbase/blob/master/vendor/gopkg.in/bblfsh/sdk.v2/uast/query/xpath/xpath.go#L30

So every time when we apply xpath via tools wrapper we compile and execute xpath.

@ajnavarro
Copy link
Contributor Author

@kuba-- but we are not failing the query if the xpath is wrong, maybe we should fail on that case.

@kuba--
Copy link
Contributor

kuba-- commented Nov 28, 2018

It should be fixed by 3rd party package: https://github.com/antchfx/xpath/blob/master/xpath.go#L139

@kuba-- kuba-- closed this as completed Nov 28, 2018
@ajnavarro ajnavarro reopened this Dec 12, 2018
@ajnavarro ajnavarro added the blocked Some other issue is blocking this label Dec 12, 2018
@ajnavarro
Copy link
Contributor Author

Related: bblfsh/sdk#332

@kuba--
Copy link
Contributor

kuba-- commented Mar 22, 2019

If we switch to the gopkg.in/xmlpath.v2at least in following test it can find the last invalid xpath (github.com/antchfx/xpath is the lib. which bblfsh uses right now):

package main

import (
	. "gopkg.in/xmlpath.v2"
	// . "github.com/antchfx/xpath"
)
func main() {
	MustCompile("//*[@role=\"Return\"]")
	MustCompile("BAD")

	MustCompile("this is a totally crap. For sure not valid @xpath") // <----
}

@kuba-- kuba-- removed their assignment Jun 4, 2019
@erizocosmico
Copy link
Contributor

What shall we do with this? @ajnavarro

@ajnavarro
Copy link
Contributor Author

we can close this as cannot be fixed from our side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Some other issue is blocking this enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants