Skip to content

Conversation

@duk3luk3
Copy link
Member

@duk3luk3 duk3luk3 commented Aug 30, 2017

marshmallow does not like empty strings in UrlFields and therefore failed validation if a mod_info.lua contained url = "".

This commit removes all None and "" values from the data before passing it to the marshmallow schema.

Fixes #31

Also contains a lua fix:

Ignore "internal" globals when parsing lua: When we execute lua scripts to extract the global variables they set, we are not interested in the things that already hang around in the globals table when the Lua VM is initialized.

Since these things also contain binary that we don't want to run through string decoding (resulting in UnicodeDecodeError), we're better off getting rid of them.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 71.995% when pulling 2596548 on duk3luk3:fix-31-mod-validation into e7c3d6c on FAForever:develop.

@Sheeo
Copy link
Member

Sheeo commented Aug 30, 2017

Don't you just want to update the schema with url = fields.Url(default="", allow_none=True)?

@duk3luk3
Copy link
Member Author

Ah, I didn't know that was possible! I didn't find that in the marshmallow docs when I was looking.

@duk3luk3
Copy link
Member Author

This doesn't actually work, and apparently the workaround is to remove all "empty" values before passing the data to marshmallow - marshmallow-code/marshmallow#167 (comment)

For a schema description tool this is kind of shite.

When we execute lua scripts to extract the global variables they set, we
are not interested in the things that already hang around in the globals
table when the Lua VM is initialized.

Since these things also contain binary that we don't want to run through
string decoding (resulting in `UnicodeDecodeError`), we're better off
getting rid of them.
marshmallow does not like empty strings in `UrlField`s and therefore
failed validation if a `mod_info.lua` contained `url = ""`.

This commit removes all `None` and `""` values from the data before
passing it to the marshmallow schema.

Fixes FAForever#31
@duk3luk3 duk3luk3 force-pushed the fix-31-mod-validation branch from 2596548 to 6687cab Compare August 31, 2017 13:56
@duk3luk3 duk3luk3 changed the title parse_mode_info: Return errors with result instead of raising parse_mod_info: Filter empty strings to marshmallow doesn't get angry Aug 31, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 72.145% when pulling 6687cab on duk3luk3:fix-31-mod-validation into e7c3d6c on FAForever:develop.

@duk3luk3
Copy link
Member Author

@Sheeo can you reproduce that without my commit to ignore the "internal" lua globals using from_lua on anything causes a UnicodeDecodeError? I don't quite understand why the python-api doesn't already suffer from this problem.

@duk3luk3
Copy link
Member Author

Here is what it takes for me to trigger the problem:

from faf.tools.lua import parse
parse.from_lua('')

results in this:

(.venv) erlacher@laprbg12 faftools(develop|…) % PYTHONSTARTUP=~/.pythonrc python                                                                                                                                            ~/git/faf/faftools
Python 3.6.2 (default, Jul 20 2017, 03:52:27) 
[GCC 7.1.1 20170630] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from faf.tools.lua import parse
>>> parse.from_lua('')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/erlacher/git/faf/faftools/faf/tools/lua/parse.py", line 31, in from_lua
    return unfold_table(lua.globals())
  File "/home/erlacher/git/faf/faftools/faf/tools/lua/parse.py", line 28, in unfold_table
    result[k] = dict(v)
  File "lupa/_lupa.pyx", line 600, in lupa._lupa._LuaObject.__getitem__ (lupa/_lupa.c:10396)
  File "lupa/_lupa.pyx", line 617, in lupa._lupa._LuaObject._getitem (lupa/_lupa.c:10593)
  File "lupa/_lupa.pyx", line 1074, in lupa._lupa.py_from_lua (lupa/_lupa.c:16639)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xc2 in position 4: invalid continuation byte
>>> 

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.

mod validation is overly strict

3 participants