Skip to content

Syntax error on assertion reinterpretation #612

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

Closed
pytestbot opened this issue Oct 8, 2014 · 12 comments
Closed

Syntax error on assertion reinterpretation #612

pytestbot opened this issue Oct 8, 2014 · 12 comments
Labels
type: bug problem that needs to be addressed

Comments

@pytestbot
Copy link
Contributor

Originally reported by: Alex Gaynor (BitBucket: alex_gaynor, GitHub: alex_gaynor)


The function with the assert statement is:

    def _ec_key_set_public_key_affine_coordinates(self, ctx, x, y):
        """
        This is a port of EC_KEY_set_public_key_affine_coordinates that was
        added in 1.0.1.

        Sets the public key point in the EC_KEY context to the affine x and y
        values.
        """

        bn_x = self._int_to_bn(x)
        bn_y = self._int_to_bn(y)

        set_func, get_func, group = (
            self._ec_key_determine_group_get_set_funcs(ctx)
        )

        point = self._lib.EC_POINT_new(group)
        assert point != self._ffi.NULL
        point = self._ffi.gc(point, self._lib.EC_POINT_free)

        with self._tmp_bn_ctx() as bn_ctx:
            check_x = self._lib.BN_CTX_get(bn_ctx)
            check_y = self._lib.BN_CTX_get(bn_ctx)

            res = set_func(group, point, bn_x, bn_y, bn_ctx)
            assert res == 1

            res = get_func(group, point, check_x, check_y, bn_ctx)
            assert res == 1

            assert (
                self._lib.BN_cmp(bn_x, check_x) == 0 and
                self._lib.BN_cmp(bn_y, check_y) == 0
            )

        res = self._lib.EC_KEY_set_public_key(ctx, point)
        assert res == 1

        res = self._lib.EC_KEY_check_key(ctx)
        assert res == 1

        return ctx

The specific assertion that's failing is the:

            assert (
                self._lib.BN_cmp(bn_x, check_x) == 0 and
                self._lib.BN_cmp(bn_y, check_y) == 0
            )

The source code that py.test is trying to compile is:

self._lib.BN_cmp(bn_x, check_x) == 0 and
self._lib.BN_cmp(bn_y, check_y) == 0
            )

which is clearly a syntax error :-) The exact error message from py.test is:

__ TestPEMSerialization.test_load_pem_ec_private_key[backend1-ec_private_key_encrypted.pem-123456] __

self = <tests.hazmat.primitives.test_serialization.TestPEMSerialization object at 0x108dd8c90>
key_file = 'ec_private_key_encrypted.pem', password = '123456'
backend = <cryptography.hazmat.backends.openssl.backend.Backend object at 0x1089b5f90>

    @pytest.mark.parametrize(
        ("key_file", "password"),
        [
            ("ec_private_key.pem", None),
            ("ec_private_key_encrypted.pem", b"123456"),
        ]
    )
    @pytest.mark.elliptic
    def test_load_pem_ec_private_key(self, key_file, password, backend):
        _skip_curve_unsupported(backend, ec.SECP256R1())
        key = load_vectors_from_file(
            os.path.join(
                "asymmetric", "PEM_Serialization", key_file),
>           lambda pemfile: load_pem_private_key(
                pemfile.read().encode(), password, backend
            )
        )

tests/hazmat/primitives/test_serialization.py:76:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/utils.py:102: in load_vectors_from_file
    return loader(vector_file)
tests/hazmat/primitives/test_serialization.py:77: in <lambda>
    pemfile.read().encode(), password, backend
cryptography/hazmat/primitives/serialization.py:59: in load_pem_private_key
    return parser_type(backend).load_object(pem)
cryptography/hazmat/primitives/serialization.py:133: in load_object
    ).private_key(self._backend)
cryptography/hazmat/primitives/asymmetric/ec.py:279: in private_key
    return backend.load_elliptic_curve_private_numbers(self)
cryptography/hazmat/backends/openssl/backend.py:881: in load_elliptic_curve_private_numbers
    ec_cdata, public.x, public.y)
../../.virtualenvs/cryptography-dev/lib/python2.7/site-packages/_pytest/assertion/reinterpret.py:40: in __init__
    self.msg = reinterpret(source, f, should_fail=True)
../../.virtualenvs/cryptography-dev/lib/python2.7/site-packages/_pytest/assertion/newinterpret.py:46: in interpret
    mod = ast.parse(source)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

source = 'self._lib.BN_cmp(bn_x, check_x) == 0 and\nself._lib.BN_cmp(bn_y, check_y) == 0\n            )'
filename = '<unknown>', mode = 'exec'

    def parse(source, filename='<unknown>', mode='exec'):
        """
        Parse the source into an AST node.
        Equivalent to compile(source, filename, mode, PyCF_ONLY_AST).
        """
>       return compile(source, filename, mode, PyCF_ONLY_AST)
E         File "<unknown>", line 1
E           self._lib.BN_cmp(bn_x, check_x) == 0 and
E                                                  ^
E       SyntaxError: invalid syntax

/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/ast.py:37: SyntaxError

I haven't been able to further minimize the failure yet.


@pytestbot
Copy link
Contributor Author

Original comment by Anatoly Bubenkov (BitBucket: bubenkoff, GitHub: bubenkoff):


having and in assert...
is it really good idea?
i would have 2 asserts instead.
i agree there's some issue in pytest, but still, assert should be simple

@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


seems like a bug in the heuristics how to find valid source code. This works fine on assert rewriting i guess, right? At some point i'd like to drop assert reinterpretation to be honest and therefore i'd like to understand why you end up using it here.

@pytestbot
Copy link
Contributor Author

Original comment by Alex Gaynor (BitBucket: alex_gaynor, GitHub: alex_gaynor):


Sorry, my post was unclear: this fails on the default assert thing; I mixed up rewriting vs. reinterpretation

@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


i still think it's for some reason using re-interpretation unless i am missing something. What platform/interp is this?

@pytestbot
Copy link
Contributor Author

Original comment by Alex Gaynor (BitBucket: alex_gaynor, GitHub: alex_gaynor):


This is CPython on OS X.

@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


2.7?

@pytestbot
Copy link
Contributor Author

Original comment by Alex Gaynor (BitBucket: alex_gaynor, GitHub: alex_gaynor):


Yes.

@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


So indeed we need to check why reinterpretation happens here and if it's a more general problem (likely). Maybe @gutworth also has something to contribute.

@pytestbot
Copy link
Contributor Author

Original comment by Benjamin Peterson (BitBucket: gutworth, GitHub: gutworth):


reinterpretation is happening because the assert is not in a test file.

@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


OK, i think we need to refine the behaviour a bit:

  • if rewrite is choosen (default), AssertionError will not be overwritten in the builtins and thus only test modules will be affected at all.
  • if reinterp is choosen, AssertionError will be overwritten and be effective for all modules
  • we might expose the rewrite file pattern as an ini option so that you can turn it on for your packages as well in an ini option

How important it is to fix the re-interpretation error originally reported i don't know -- i would think reinterpretation is hardly used and i'd like to phase it out eventually.

@pytestbot
Copy link
Contributor Author

Original comment by Alex Gaynor (BitBucket: alex_gaynor, GitHub: alex_gaynor):


This showed up again today: https://jenkins.cryptography.io/job/cryptography-pr-experimental/TOXENV=py34,label=windows64/2388/console

@pytestbot pytestbot added the type: bug problem that needs to be addressed label Jun 15, 2015
@flub
Copy link
Member

flub commented Sep 5, 2016

We have now removed assertion reinterpretation so I think this can be closed. There is now a pytest.register_assert_rewrite() method to mark any modules which need to be re-written if they are not in plugins (or conftest.py files which are also plugins).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug problem that needs to be addressed
Projects
None yet
Development

No branches or pull requests

2 participants