validator: refactor errors to improve information and clarity

validation/src/validator.rs

@@ -21,23 +21,31 @@ fn source_has_illegal_keywords(source:&str)->bool{
 	source.find("getfenv").is_some()||source.find("require").is_some()
 }
 
+fn hash_source(source:&str)->String{
+	let mut hasher=siphasher::sip::SipHasher::new();
+	std::hash::Hasher::write(&mut hasher,source.as_bytes());
+	let hash=std::hash::Hasher::finish(&hasher);
+	format!("{:016x}",hash)
+}
+
 #[allow(dead_code)]
 #[derive(Debug)]
 pub enum ValidateError{
-	Flagged,
-	Blocked,
-	NotAllowed,
-	Get(rbx_asset::cookie::GetError),
-	ReadDom(ReadDomError),
-	ApiGetScriptPolicy(submissions_api::types::SingleItemError),
+	ScriptFlaggedIllegalKeyword,
+	ScriptBlocked(Option<submissions_api::types::ScriptID>),
+	ScriptNotYetReviewed(Option<submissions_api::types::ScriptID>),
+	ModelFileDownload(rbx_asset::cookie::GetError),
+	ModelFileDecode(ReadDomError),
+	ApiGetScriptPolicyFromHash(submissions_api::types::SingleItemError),
 	ApiGetScript(submissions_api::Error),
 	ApiCreateScript(submissions_api::Error),
 	ApiCreateScriptPolicy(submissions_api::Error),
+	ApiGetScriptFromHash(submissions_api::types::SingleItemError),
 	ApiUpdateSubmissionModel(submissions_api::Error),
 	ApiActionSubmissionValidate(submissions_api::Error),
-	WriteDom(rbx_binary::EncodeError),
-	Upload(rbx_asset::cookie::UploadError),
-	Create(rbx_asset::cookie::CreateError),
+	ModelFileEncode(rbx_binary::EncodeError),
+	AssetUpload(rbx_asset::cookie::UploadError),
+	AssetCreate(rbx_asset::cookie::CreateError),
 }
 impl std::fmt::Display for ValidateError{
 	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
@@ -89,10 +97,10 @@ impl Validator{
 		let data=self.roblox_cookie.get_asset(rbx_asset::cookie::GetAssetRequest{
 			asset_id:validate_info.ModelID,
 			version:Some(validate_info.ModelVersion),
-		}).await.map_err(ValidateError::Get)?;
+		}).await.map_err(ValidateError::ModelFileDownload)?;
 
 		// decode dom (slow!)
-		let mut dom=read_dom(&mut std::io::Cursor::new(data)).map_err(ValidateError::ReadDom)?;
+		let mut dom=read_dom(&mut std::io::Cursor::new(data)).map_err(ValidateError::ModelFileDecode)?;
 
 		/* VALIDATE MAP */
 
@@ -105,7 +113,7 @@ impl Validator{
 					// check the source for illegal keywords
 					if source_has_illegal_keywords(source){
 						// immediately abort
-						return Err(ValidateError::Flagged);
+						return Err(ValidateError::ScriptFlaggedIllegalKeyword);
 					}
 					// associate a name and policy with the source code
 					// policy will be fetched from the database to replace the default policy
@@ -121,14 +129,12 @@ impl Validator{
 		futures::stream::iter(script_map.iter_mut().map(Ok))
 		.try_for_each_concurrent(Some(SCRIPT_CONCURRENCY),|(source,NamePolicy{policy,name})|async{
 			// get the hash
-			let mut hasher=siphasher::sip::SipHasher::new();
-			std::hash::Hasher::write(&mut hasher,source.as_bytes());
-			let hash=std::hash::Hasher::finish(&hasher);
+			let hash=hash_source(source.as_str());
 
 			// fetch the script policy
 			let script_policy=self.api.get_script_policy_from_hash(submissions_api::types::HashRequest{
-				hash:format!("{:016x}",hash).as_str(),
-			}).await.map_err(ValidateError::ApiGetScriptPolicy)?;
+				hash:hash.as_str(),
+			}).await.map_err(ValidateError::ApiGetScriptPolicyFromHash)?;
 
 			// write the policy to the script_map, fetching the replacement code if necessary
 			if let Some(script_policy)=script_policy{
@@ -170,10 +176,22 @@ impl Validator{
 			if let Some(script)=dom.get_by_ref_mut(script_ref){
 				if let Some(rbx_dom_weak::types::Variant::String(source))=script.properties.get_mut("Source"){
 					match script_map.get(source.as_str()).map(|p|&p.policy){
-						Some(Policy::Blocked)=>return Err(ValidateError::Blocked),
+						Some(Policy::Blocked)=>{
+							let hash=hash_source(source.as_str());
+							let script=self.api.get_script_from_hash(submissions_api::types::HashRequest{
+								hash:hash.as_str(),
+							}).await.map_err(ValidateError::ApiGetScriptFromHash)?;
+							return Err(ValidateError::ScriptBlocked(script.map(|s|s.ID)));
+						},
 						None
 						|Some(Policy::None)
-						=>return Err(ValidateError::NotAllowed),
+						=>{
+							let hash=hash_source(source.as_str());
+							let script=self.api.get_script_from_hash(submissions_api::types::HashRequest{
+								hash:hash.as_str(),
+							}).await.map_err(ValidateError::ApiGetScriptFromHash)?;
+							return Err(ValidateError::ScriptNotYetReviewed(script.map(|s|s.ID)));
+						},
 						Some(Policy::Allowed)=>(),
 						Some(Policy::Delete)=>{
 							modified=true;
@@ -195,7 +213,7 @@ impl Validator{
 		if modified{
 			// serialize model (slow!)
 			let mut data=Vec::new();
-			rbx_binary::to_writer(&mut data,&dom,dom.root().children()).map_err(ValidateError::WriteDom)?;
+			rbx_binary::to_writer(&mut data,&dom,dom.root().children()).map_err(ValidateError::ModelFileEncode)?;
 
 			// upload a model lol
 			let model_id=if let Some(model_id)=validate_info.ValidatedModelID{
@@ -207,7 +225,7 @@ impl Validator{
 					ispublic:None,
 					allowComments:None,
 					groupId:None,
-				},data).await.map_err(ValidateError::Upload)?;
+				},data).await.map_err(ValidateError::AssetUpload)?;
 
 				response.AssetId
 			}else{
@@ -218,7 +236,7 @@ impl Validator{
 					ispublic:true,
 					allowComments:true,
 					groupId:None,
-				},data).await.map_err(ValidateError::Create)?;
+				},data).await.map_err(ValidateError::AssetCreate)?;
 
 				response.AssetId
 			};