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

TRY/except ignores the typespec of its function argument #1514

Closed
Siskin-Bot opened this issue Feb 15, 2020 · 1 comment
Closed

TRY/except ignores the typespec of its function argument #1514

Siskin-Bot opened this issue Feb 15, 2020 · 1 comment

Comments

@Siskin-Bot
Copy link
Collaborator

Submitted by: BrianH

When you pass a function as the exception handler of TRY/except, that function should have one regular argument, which will be the error that is being handled. However, you don't have to specify a typespec for that argument despite the fact that normally arguments without typespecs don't accept error! values. This is because TRY/except doesn't do the type testing that a normal function call does - it passes in the error! argument anyways.

For that matter, the function doesn't need to take an argument at all, or can take multiple arguments: APPLY/only semantics are used for the function arguments, so none is passed to any additional arguments. This wouldn't necessarily be a problem, but when combined with the lack of type checking it gives us an easy way to crash R3.

The advantage to the lack of type checking is that we don't need to write a verbose type spec in our handlers. The disadvantage is that you can easily crash R3 if you pass the wrong native function as a handler.

It would be a good idea to at least do the type checking when calling native functions, including natives, actions, ops and commands. You can't crash R3 with a regular function or closure in this case, so the type checking isn't as necessary. It might not be a good idea to disallow native functions altogether, but that could be an acceptable alternative to selective type checking if necessary.

; Some legit uses:
>> try/except [1 / 0] func [e] [print mold e]
make error! [
    code: 400
    type: 'Math
    id: 'zero-divide
    arg1: none
    arg2: none
    arg3: none
    near: [/ 0]
    where: [/ try]
]
>> try/except [1 / 0] func [e] [type? e]
== error!
; Note that the lack of typespec for the argument may not be technically correct, but looks better.

; Iffy but acceptable examples (don't crash)
>> try/except [1 / 0] func [e [integer!]] [type? e]
== error!
>> try/except [1 / 0] :load
** Script error: file-type? does not allow error! for its file argument
** Where: case applier try
** Near: case [
    block? source [
        return map-each x source ...
; Note that LOAD errors out on the inside, but doesn't crash R3

; Some correct and semi-useful natives that accept error args
>> try/except [1 / 0] :mold
== {make error! [
    code: 400
    type: 'Math
    id: 'zero-divide
    arg1: none
    arg2: none
    arg3: none
    near: [/ 0]
    where: [/ try]
]}
>> try/except [1 / 0] :form
== {** Math error: attempt to divide by zero
** Where: / try
** Near: / 0
}
>> try/except [1 / 0] :print
** Math error: attempt to divide by zero
** Where: / try
** Near: / 0
; If natives were excluded then these wouldn't work, but that might not be too bad.

; And here is the problem: A native that doesn't accept errors.
>> try/except [1 / 0] :add
; R3 crashes without even a system exception, the process just dies.

Imported from: CureCode [ Version: alpha 97 Type: Bug Platform: All Category: Native Reproduce: Always Fixed-in:none ]
Imported from: metaeducation#1514

Comments:

Rebolbot commented on Mar 23, 2014:

Submitted by: abolka

In the core-tests suite.


Rebolbot commented on Aug 27, 2014:

Submitted by: szeng

"despite the fact that normally arguments without typespecs don't accept error! values"

This doesn't seem to be true (any more)

>> a: make error! [type: 'user id: 'message arg1: "hello"] 
** user error: "hello"
>> ?? a
a: make error! [
    code: 800
    type: 'user
    id: 'message
    arg1: "hello"
    arg2: none
    arg3: none
    near: none
    where: none
]
** user error: "hello"
>> c: func [a][if error? a [print "a:" a]]
>> c a
a:
** user error: "hello"

Rebolbot commented on Aug 27, 2014:

Submitted by: szeng

I think we should check the type of the argument as we do for other functions.


Rebolbot commented on Aug 27, 2014:

Submitted by: abolka

> I think we should check the type of the argument as we do for other functions.

Agreed. I have a fix for that implemented which I'll push in a minute.


Rebolbot commented on Aug 27, 2014:

Submitted by: abolka

Pull request submitted:

https://github.com/rebol/rebol/pull/227

Rebolbot commented on Aug 27, 2014:

Submitted by: szeng

I was working on a similar fix. But yours is back compatiable, mine was more strictive on the number of arguments.
For the reference:

https://gist.github.com/zsx/9c8628db9bcded698b5a

Rebolbot added on Jan 12, 2016


Oldes added a commit to Oldes/Rebol3 that referenced this issue on Jun 8, 2018:
ATRONIX: Type-check call of TRY/except handler


@Oldes
Copy link
Owner

Oldes commented May 29, 2020

> try/except [1 / 0] :add
** Script error: action! does not allow error! for its value1 argument
** Where: try
** Near: try/except [1 / 0] :add

@Oldes Oldes closed this as completed May 29, 2020
Oldes added a commit to Oldes/Rebol3 that referenced this issue May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants