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

[null safety] Possible null safety hole with structs and/or monomorphs #10147

Closed
jonasmalacofilho opened this issue Mar 1, 2021 · 3 comments · Fixed by #10850
Closed

[null safety] Possible null safety hole with structs and/or monomorphs #10147

jonasmalacofilho opened this issue Mar 1, 2021 · 3 comments · Fixed by #10850
Labels

Comments

@jonasmalacofilho
Copy link
Member

I saw a post on reddit that suggested that using structs can sometimes evade the compile-time null safety checks.

typedef HasAString = {
  theString : String
}
// [...]
var has : HasAString = { theString: null };  // compiles with @:nullSafety(Strict)

I have also been able to reproduce it without the typedef, although with a more obvious intermediate { text : Unknown<0> } monomorph:

var tmp = { text: null };
var typed_receiver: { text : String } = tmp;  // compiles with @:nullSafety(Strict)
trace(typed_receiver.text.length);  // throws at runtime: TypeError: null has no properties

Oddly, the follow still isn't allowed to compile, as desired:

var typed_struct: { text: String } = { text: null };  // ERROR: Null safety: Cannot assign nullable value here.

Related: r/haxe: Null safety in structs
Related: original example, on try-haxe
Related: other examples, on try-haxe

@RealyUniqueName RealyUniqueName added this to the Bugs milestone Mar 4, 2021
@RblSb
Copy link
Member

RblSb commented Nov 25, 2021

More cases:

typedef HasAString = {
	text:String
}
typedef HasANullString = {
	text:Null<String>
}

@:nullSafety(StrictThreaded)
class Main {
	static function main() {
		final has:HasAString = {text: null};

		final tmp = {text: null};
		final typed:HasAString = tmp;

		final tmp = {text: null};
		final typed:{text:String} = tmp;

		final tmp = {text: null};
		final arr:Array<String> = [tmp.text];

		// should pass
		final tmp = {text: null};
		final typed:{text:Null<String>} = tmp;
		final typed2:HasANullString = tmp;
		final typed3:HasANullString = typed;
	}
}

My solution for nullSafety.ml, in method private check_expr e:

| TObjectDecl fields ->
	List.iter (fun ((name,pos,_), e) ->
		begin match e.etype with
			(* Field type still Unknown *)
			| TMono r when r.tm_type = None -> ()
			| _ ->
				if not (self#can_pass_expr e e.etype e.epos) then begin
					self#error ("Cannot use nullable value as an field of type " ^ (str_type e.etype)) [e.epos; pos];
				end;
		end;
		self#check_expr e
	) fields

It will report it like that, which is not that good, maybe.
image

@RealyUniqueName
Copy link
Member

RealyUniqueName commented Nov 25, 2021

It happens because Haxe doesn't add Null<> to tmp.text while typing. Instead tmp.text becomes just a monomorph (Unknown instead of Null<Unknown>).
I remember discussing this behavior in relation to local vars (var s = null also typed without Null<>) somewhere in issues, but can't find it now.

@RblSb
Copy link
Member

RblSb commented Nov 25, 2021

Yep, this will solve it too. Would be nice to change typing, so as example "Left operand is not nullable" warnings can be added to null coalescing, without false positive warnings for var a = null; a ?? 0;

RblSb added a commit to RblSb/haxe that referenced this issue Nov 16, 2022
Simn pushed a commit that referenced this issue Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants