xml: make structs serializable, but enforce contracts#5283
xml: make structs serializable, but enforce contracts#5283mflatt merged 2 commits intoracket:masterfrom
Conversation
|
Thank you! |
Fix `xexpr/c`, `xexpr?`, `correct-xexpr?`, and `validate-xexpr` to reject instances of the `pcdata` struct, consistent with the documentation and other parts of the implementation. Trying to use `pcdata` structs as X-expressions was already very broken, e.g.: ``` $ racket Welcome to Racket v8.17 [cs]. > (require xml) > (xexpr->string (pcdata #f #f "pcdata")) "" ```
6fa1e92 to
48df9e1
Compare
|
I've added docs and tests. While working on the tests, I found that a bug in Since this is so broken, I'm hoping it will work to make this change, but I've put it in a separate commit in case someone is somehow relying on the current behavior and we have to revert. I've put that commit before the one adding serialization, because, when the structs become serializable, the module path providing their deserialize info becomes part of the API. Having tried it both ways, it's much nicer to move from (require (only-in (submod "core.rkt" deserialize-info) deserialize-info:p-i-v0))
(provide deserialize-info:p-i-v0)rather than: (provide deserialize-info:p-i-v0)
(define deserialize-info:p-i-v0
(dynamic-require (module-path-index-join '(submod "." deserialize-info)
mpi-for-contracts)
'deserialize-info:p-i-v0))I also included some other doc tweaks with the serialization commit. The addition of |
| (require 'serialization-support) | ||
| (define-runtime-module-path-index deserialize-info-mpi '(submod "." deserialize-info)) | ||
| (module+ deserialize-info | ||
| (define-runtime-module-path-index mpi-for-contracts "structures.rkt") |
There was a problem hiding this comment.
I wanted to double-check that this doesn't need to be:
(define-runtime-module-path mpi-for-contracts xml/private/structures)There was a problem hiding this comment.
Using '(submod "." deserialize-info) seems fine to me.
There was a problem hiding this comment.
Oh, I was thinking about the one that is currently (define-runtime-module-path-index mpi-for-contracts "structures.rkt"). I vaguely remembered that some ways of mixing styles of references could cause trouble for raco exe in some cases, but I've never actually encountered those problems myself, so I'm not very clear about it.
There was a problem hiding this comment.
I guess if someone referred to "core.rkt" through a file path instead of a module path, then there would be a difference for both of those relative module references, and there could be a mismatch. For that case, using relative paths as here might be better, or it might be worse, depending on the context. In any case, referencing "core.rkt" both through a file path and a module path would create other (related) problems with raco exe, and I don't think we need to worry about it.
Also, tweak docs and add `no-external-dtd`. Increment version to 8.17.0.5. Related to: racket/scribble#498 Related to: racket#5282 Related to: racket/rhombus#644
48df9e1 to
81b9fbc
Compare
|
This looks fine to me, so I'll merge soon if there are no objections. |
|
Wow! That's one heck of a commit. I've so much to learn. Thanks again. |
This addresses the issue with #5282 that could allow evading contracts.
Since an X-expression can contain a
locationin asource-startorsource-stop, I needed to add serialization support toxml/private/structuresin addition toxml/private/core. At that point, it seemed to make sense to make the structs that aren't used in X-expressions serializable, too.One caveat is that contract violations on deserialization will blame the
deserialize-infosubmodules. That's not theoretically ideal, but seems practically fine.Still needs docs and tests.
Related to: racket/scribble#498
Related to: #5282