Changeset 213322 in webkit
- Timestamp:
- Mar 2, 2017 4:18:44 PM (7 years ago)
- Location:
- trunk/Source/WebCore
- Files:
-
- 6 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebCore/ChangeLog
r213320 r213322 1 2017-03-02 Alex Christensen <achristensen@webkit.org> 2 3 Modernize ContentExtensionParser 4 https://bugs.webkit.org/show_bug.cgi?id=169106 5 6 Reviewed by Andy Estes. 7 8 We were returning std::error_code and passing the return value as a parameter reference. 9 because we wrote this code before we had WTF::Expected. 10 11 No change in behavior, verified by many API tests. 12 13 * contentextensions/ContentExtensionCompiler.cpp: 14 (WebCore::ContentExtensions::compileRuleList): 15 * contentextensions/ContentExtensionParser.cpp: 16 (WebCore::ContentExtensions::getDomainList): 17 (WebCore::ContentExtensions::loadTrigger): 18 (WebCore::ContentExtensions::loadAction): 19 (WebCore::ContentExtensions::loadRule): 20 (WebCore::ContentExtensions::loadEncodedRules): 21 (WebCore::ContentExtensions::parseRuleList): 22 * contentextensions/ContentExtensionParser.h: 23 * contentextensions/ContentExtensionRule.cpp: 24 (WebCore::ContentExtensions::ContentExtensionRule::ContentExtensionRule): 25 * contentextensions/ContentExtensionRule.h: 26 1 27 2017-03-02 Ryan Haddad <ryanhaddad@apple.com> 2 28 -
trunk/Source/WebCore/contentextensions/ContentExtensionCompiler.cpp
r194496 r213322 206 206 } 207 207 208 std::error_code compileRuleList(ContentExtensionCompilationClient& client, String&& rule List)208 std::error_code compileRuleList(ContentExtensionCompilationClient& client, String&& ruleJSON) 209 209 { 210 Vector<ContentExtensionRule> parsedRuleList; 211 auto parserError = parseRuleList(ruleList, parsedRuleList); 212 ruleList = String(); 213 if (parserError) 214 return parserError; 210 auto ruleList = parseRuleList(WTFMove(ruleJSON)); 211 if (!ruleList.hasValue()) 212 return ruleList.error(); 213 Vector<ContentExtensionRule> parsedRuleList = WTFMove(ruleList.value()); 215 214 216 215 #if CONTENT_EXTENSIONS_PERFORMANCE_REPORTING -
trunk/Source/WebCore/contentextensions/ContentExtensionParser.cpp
r209906 r213322 41 41 #include <JavaScriptCore/VM.h> 42 42 #include <wtf/CurrentTime.h> 43 #include <wtf/Expected.h> 43 44 #include <wtf/text/WTFString.h> 44 45 … … 59 60 } 60 61 61 static std::error_code getDomainList(ExecState& exec, const JSObject* arrayObject, Vector<String>& vector) 62 { 63 VM& vm = exec.vm(); 64 auto scope = DECLARE_THROW_SCOPE(vm); 65 66 ASSERT(vector.isEmpty()); 62 static Expected<Vector<String>, std::error_code> getDomainList(ExecState& exec, const JSObject* arrayObject) 63 { 64 VM& vm = exec.vm(); 65 auto scope = DECLARE_THROW_SCOPE(vm); 66 67 67 if (!arrayObject || !isJSArray(arrayObject)) 68 return ContentExtensionError::JSONInvalidDomainList;68 return makeUnexpected(ContentExtensionError::JSONInvalidDomainList); 69 69 const JSArray* array = jsCast<const JSArray*>(arrayObject); 70 70 71 Vector<String> domains; 71 72 unsigned length = array->length(); 72 73 for (unsigned i = 0; i < length; ++i) { 73 74 const JSValue value = array->getIndex(&exec, i); 74 75 if (scope.exception() || !value.isString()) 75 return ContentExtensionError::JSONInvalidDomainList;76 return makeUnexpected(ContentExtensionError::JSONInvalidDomainList); 76 77 77 78 // Domains should be punycode encoded lower case. 78 79 const String& domain = asString(value)->value(&exec); 79 80 if (domain.isEmpty()) 80 return ContentExtensionError::JSONInvalidDomainList;81 return makeUnexpected(ContentExtensionError::JSONInvalidDomainList); 81 82 if (!containsOnlyASCIIWithNoUppercase(domain)) 82 return ContentExtensionError::JSONDomainNotLowerCaseASCII;83 vector.append(domain);84 } 85 return { };83 return makeUnexpected(ContentExtensionError::JSONDomainNotLowerCaseASCII); 84 domains.append(domain); 85 } 86 return WTFMove(domains); 86 87 } 87 88 … … 118 119 } 119 120 120 static std::error_code loadTrigger(ExecState& exec, const JSObject& ruleObject, Trigger& trigger)121 static Expected<Trigger, std::error_code> loadTrigger(ExecState& exec, const JSObject& ruleObject) 121 122 { 122 123 VM& vm = exec.vm(); … … 125 126 const JSValue triggerObject = ruleObject.get(&exec, Identifier::fromString(&exec, "trigger")); 126 127 if (!triggerObject || scope.exception() || !triggerObject.isObject()) 127 return ContentExtensionError::JSONInvalidTrigger;128 return makeUnexpected(ContentExtensionError::JSONInvalidTrigger); 128 129 129 130 const JSValue urlFilterObject = triggerObject.get(&exec, Identifier::fromString(&exec, "url-filter")); 130 131 if (!urlFilterObject || scope.exception() || !urlFilterObject.isString()) 131 return ContentExtensionError::JSONInvalidURLFilterInTrigger;132 return makeUnexpected(ContentExtensionError::JSONInvalidURLFilterInTrigger); 132 133 133 134 String urlFilter = asString(urlFilterObject)->value(&exec); 134 135 if (urlFilter.isEmpty()) 135 return ContentExtensionError::JSONInvalidURLFilterInTrigger; 136 136 return makeUnexpected(ContentExtensionError::JSONInvalidURLFilterInTrigger); 137 138 Trigger trigger; 137 139 trigger.urlFilter = urlFilter; 138 140 … … 145 147 auto typeFlagsError = getTypeFlags(exec, resourceTypeValue, trigger.flags, readResourceType); 146 148 if (typeFlagsError) 147 return typeFlagsError;149 return makeUnexpected(typeFlagsError); 148 150 } else if (!resourceTypeValue.isUndefined()) 149 return ContentExtensionError::JSONInvalidTriggerFlagsArray;151 return makeUnexpected(ContentExtensionError::JSONInvalidTriggerFlagsArray); 150 152 151 153 const JSValue loadTypeValue = triggerObject.get(&exec, Identifier::fromString(&exec, "load-type")); … … 153 155 auto typeFlagsError = getTypeFlags(exec, loadTypeValue, trigger.flags, readLoadType); 154 156 if (typeFlagsError) 155 return typeFlagsError;157 return makeUnexpected(typeFlagsError); 156 158 } else if (!loadTypeValue.isUndefined()) 157 return ContentExtensionError::JSONInvalidTriggerFlagsArray; 158 159 const JSValue ifDomain = triggerObject.get(&exec, Identifier::fromString(&exec, "if-domain")); 160 if (!scope.exception() && ifDomain.isObject()) { 161 auto ifDomainError = getDomainList(exec, asObject(ifDomain), trigger.domains); 162 if (ifDomainError) 163 return ifDomainError; 159 return makeUnexpected(ContentExtensionError::JSONInvalidTriggerFlagsArray); 160 161 const JSValue ifDomainValue = triggerObject.get(&exec, Identifier::fromString(&exec, "if-domain")); 162 if (!scope.exception() && ifDomainValue.isObject()) { 163 auto ifDomain = getDomainList(exec, asObject(ifDomainValue)); 164 if (!ifDomain.hasValue()) 165 return makeUnexpected(ifDomain.error()); 166 trigger.domains = WTFMove(ifDomain.value()); 164 167 if (trigger.domains.isEmpty()) 165 return ContentExtensionError::JSONInvalidDomainList;168 return makeUnexpected(ContentExtensionError::JSONInvalidDomainList); 166 169 ASSERT(trigger.domainCondition == Trigger::DomainCondition::None); 167 170 trigger.domainCondition = Trigger::DomainCondition::IfDomain; 168 } else if (!ifDomain .isUndefined())169 return ContentExtensionError::JSONInvalidDomainList;170 171 const JSValue unlessDomain = triggerObject.get(&exec, Identifier::fromString(&exec, "unless-domain"));172 if (!scope.exception() && unlessDomain .isObject()) {171 } else if (!ifDomainValue.isUndefined()) 172 return makeUnexpected(ContentExtensionError::JSONInvalidDomainList); 173 174 const JSValue unlessDomainValue = triggerObject.get(&exec, Identifier::fromString(&exec, "unless-domain")); 175 if (!scope.exception() && unlessDomainValue.isObject()) { 173 176 if (trigger.domainCondition != Trigger::DomainCondition::None) 174 return ContentExtensionError::JSONUnlessAndIfDomain; 175 auto unlessDomainError = getDomainList(exec, asObject(unlessDomain), trigger.domains); 176 if (unlessDomainError) 177 return unlessDomainError; 177 return makeUnexpected(ContentExtensionError::JSONUnlessAndIfDomain); 178 auto unlessDomain = getDomainList(exec, asObject(unlessDomainValue)); 179 if (!unlessDomain.hasValue()) 180 return makeUnexpected(unlessDomain.error()); 181 trigger.domains = WTFMove(unlessDomain.value()); 178 182 if (trigger.domains.isEmpty()) 179 return ContentExtensionError::JSONInvalidDomainList;183 return makeUnexpected(ContentExtensionError::JSONInvalidDomainList); 180 184 trigger.domainCondition = Trigger::DomainCondition::UnlessDomain; 181 } else if (!unlessDomain .isUndefined())182 return ContentExtensionError::JSONInvalidDomainList;183 184 return { };185 } else if (!unlessDomainValue.isUndefined()) 186 return makeUnexpected(ContentExtensionError::JSONInvalidDomainList); 187 188 return WTFMove(trigger); 185 189 } 186 190 … … 194 198 } 195 199 196 static std::error_code loadAction(ExecState& exec, const JSObject& ruleObject, Action& action, bool& validSelector) 197 { 198 VM& vm = exec.vm(); 199 auto scope = DECLARE_THROW_SCOPE(vm); 200 201 validSelector = true; 200 static Expected<std::optional<Action>, std::error_code> loadAction(ExecState& exec, const JSObject& ruleObject) 201 { 202 VM& vm = exec.vm(); 203 auto scope = DECLARE_THROW_SCOPE(vm); 204 202 205 const JSValue actionObject = ruleObject.get(&exec, Identifier::fromString(&exec, "action")); 203 206 if (!actionObject || scope.exception() || !actionObject.isObject()) 204 return ContentExtensionError::JSONInvalidAction;207 return makeUnexpected(ContentExtensionError::JSONInvalidAction); 205 208 206 209 const JSValue typeObject = actionObject.get(&exec, Identifier::fromString(&exec, "type")); 207 210 if (!typeObject || scope.exception() || !typeObject.isString()) 208 return ContentExtensionError::JSONInvalidActionType;211 return makeUnexpected(ContentExtensionError::JSONInvalidActionType); 209 212 210 213 String actionType = asString(typeObject)->value(&exec); 211 214 212 215 if (actionType == "block") 213 action = ActionType::BlockLoad;214 elseif (actionType == "ignore-previous-rules")215 action = ActionType::IgnorePreviousRules;216 elseif (actionType == "block-cookies")217 action = ActionType::BlockCookies;218 elseif (actionType == "css-display-none") {216 return {{ActionType::BlockLoad}}; 217 if (actionType == "ignore-previous-rules") 218 return {{ActionType::IgnorePreviousRules}}; 219 if (actionType == "block-cookies") 220 return {{ActionType::BlockCookies}}; 221 if (actionType == "css-display-none") { 219 222 JSValue selector = actionObject.get(&exec, Identifier::fromString(&exec, "selector")); 220 223 if (!selector || scope.exception() || !selector.isString()) 221 return ContentExtensionError::JSONInvalidCSSDisplayNoneActionType;224 return makeUnexpected(ContentExtensionError::JSONInvalidCSSDisplayNoneActionType); 222 225 223 226 String selectorString = asString(selector)->value(&exec); 224 227 if (!isValidSelector(selectorString)) { 225 228 // Skip rules with invalid selectors to be backwards-compatible. 226 validSelector = false; 227 return { }; 229 return {std::nullopt}; 228 230 } 229 action = Action(ActionType::CSSDisplayNoneSelector, selectorString); 230 } else if (actionType == "make-https") { 231 action = ActionType::MakeHTTPS; 232 } else 233 return ContentExtensionError::JSONInvalidActionType; 234 235 return { }; 236 } 237 238 static std::error_code loadRule(ExecState& exec, const JSObject& ruleObject, Vector<ContentExtensionRule>& ruleList) 239 { 240 Trigger trigger; 241 auto triggerError = loadTrigger(exec, ruleObject, trigger); 242 if (triggerError) 243 return triggerError; 244 245 Action action; 246 bool validSelector; 247 auto actionError = loadAction(exec, ruleObject, action, validSelector); 248 if (actionError) 249 return actionError; 250 251 if (validSelector) 252 ruleList.append(ContentExtensionRule(trigger, action)); 253 return { }; 254 } 255 256 static std::error_code loadEncodedRules(ExecState& exec, const String& rules, Vector<ContentExtensionRule>& ruleList) 231 return {Action(ActionType::CSSDisplayNoneSelector, selectorString)}; 232 } 233 if (actionType == "make-https") 234 return {{ActionType::MakeHTTPS}}; 235 return makeUnexpected(ContentExtensionError::JSONInvalidActionType); 236 } 237 238 static Expected<std::optional<ContentExtensionRule>, std::error_code> loadRule(ExecState& exec, const JSObject& ruleObject) 239 { 240 auto trigger = loadTrigger(exec, ruleObject); 241 if (!trigger.hasValue()) 242 return makeUnexpected(trigger.error()); 243 244 auto action = loadAction(exec, ruleObject); 245 if (!action.hasValue()) 246 return makeUnexpected(action.error()); 247 248 if (action.value()) 249 return {{{WTFMove(trigger.value()), WTFMove(action.value().value())}}}; 250 251 return {std::nullopt}; 252 } 253 254 static Expected<Vector<ContentExtensionRule>, std::error_code> loadEncodedRules(ExecState& exec, String&& ruleJSON) 257 255 { 258 256 VM& vm = exec.vm(); … … 260 258 261 259 // FIXME: JSONParse should require callbacks instead of an ExecState. 262 const JSValue decodedRules = JSONParse(&exec, rule s);260 const JSValue decodedRules = JSONParse(&exec, ruleJSON); 263 261 264 262 if (scope.exception() || !decodedRules) 265 return ContentExtensionError::JSONInvalid;263 return makeUnexpected(ContentExtensionError::JSONInvalid); 266 264 267 265 if (!decodedRules.isObject()) 268 return ContentExtensionError::JSONTopLevelStructureNotAnObject;266 return makeUnexpected(ContentExtensionError::JSONTopLevelStructureNotAnObject); 269 267 270 268 const JSObject* topLevelObject = decodedRules.toObject(&exec); 271 269 if (!topLevelObject || scope.exception()) 272 return ContentExtensionError::JSONTopLevelStructureNotAnObject;270 return makeUnexpected(ContentExtensionError::JSONTopLevelStructureNotAnObject); 273 271 274 272 if (!isJSArray(topLevelObject)) 275 return ContentExtensionError::JSONTopLevelStructureNotAnArray;273 return makeUnexpected(ContentExtensionError::JSONTopLevelStructureNotAnArray); 276 274 277 275 const JSArray* topLevelArray = jsCast<const JSArray*>(topLevelObject); 278 276 279 Vector<ContentExtensionRule> localRuleList;277 Vector<ContentExtensionRule> ruleList; 280 278 281 279 unsigned length = topLevelArray->length(); 282 280 const unsigned maxRuleCount = 50000; 283 281 if (length > maxRuleCount) 284 return ContentExtensionError::JSONTooManyRules;282 return makeUnexpected(ContentExtensionError::JSONTooManyRules); 285 283 for (unsigned i = 0; i < length; ++i) { 286 284 const JSValue value = topLevelArray->getIndex(&exec, i); 287 285 if (scope.exception() || !value) 288 return ContentExtensionError::JSONInvalidObjectInTopLevelArray;286 return makeUnexpected(ContentExtensionError::JSONInvalidObjectInTopLevelArray); 289 287 290 288 const JSObject* ruleObject = value.toObject(&exec); 291 289 if (!ruleObject || scope.exception()) 292 return ContentExtensionError::JSONInvalidRule; 293 294 auto error = loadRule(exec, *ruleObject, localRuleList); 295 if (error) 296 return error; 297 } 298 299 ruleList = WTFMove(localRuleList); 300 return { }; 301 } 302 303 std::error_code parseRuleList(const String& rules, Vector<ContentExtensionRule>& ruleList) 290 return makeUnexpected(ContentExtensionError::JSONInvalidRule); 291 292 auto rule = loadRule(exec, *ruleObject); 293 if (!rule.hasValue()) 294 return makeUnexpected(rule.error()); 295 if (rule.value()) 296 ruleList.append(*rule.value()); 297 } 298 299 return WTFMove(ruleList); 300 } 301 302 Expected<Vector<ContentExtensionRule>, std::error_code> parseRuleList(String&& ruleJSON) 304 303 { 305 304 #if CONTENT_EXTENSIONS_PERFORMANCE_REPORTING … … 312 311 313 312 ExecState* exec = globalObject->globalExec(); 314 auto error = loadEncodedRules(*exec, rules, ruleList);313 auto ruleList = loadEncodedRules(*exec, WTFMove(ruleJSON)); 315 314 316 315 vm = nullptr; 317 316 318 if ( error)319 return error;320 321 if (ruleList .isEmpty())322 return ContentExtensionError::JSONContainsNoRules;317 if (!ruleList.hasValue()) 318 return makeUnexpected(ruleList.error()); 319 320 if (ruleList->isEmpty()) 321 return makeUnexpected(ContentExtensionError::JSONContainsNoRules); 323 322 324 323 #if CONTENT_EXTENSIONS_PERFORMANCE_REPORTING … … 327 326 #endif 328 327 329 return { };328 return WTFMove(*ruleList); 330 329 } 331 330 -
trunk/Source/WebCore/contentextensions/ContentExtensionParser.h
r208179 r213322 29 29 30 30 #include <system_error> 31 #include <wtf/Expected.h> 31 32 #include <wtf/Forward.h> 32 33 #include <wtf/Vector.h> … … 38 39 class ContentExtensionRule; 39 40 40 std::error_code parseRuleList(const String& rules, Vector<ContentExtensionRule>&);41 Expected<Vector<ContentExtensionRule>, std::error_code> parseRuleList(String&&); 41 42 42 43 } // namespace ContentExtensions -
trunk/Source/WebCore/contentextensions/ContentExtensionRule.cpp
r194386 r213322 33 33 namespace ContentExtensions { 34 34 35 ContentExtensionRule::ContentExtensionRule( const Trigger& trigger, const Action& action)36 : m_trigger( trigger)37 , m_action( action)35 ContentExtensionRule::ContentExtensionRule(Trigger&& trigger, Action&& action) 36 : m_trigger(WTFMove(trigger)) 37 , m_action(WTFMove(action)) 38 38 { 39 39 ASSERT(!m_trigger.urlFilter.isEmpty()); -
trunk/Source/WebCore/contentextensions/ContentExtensionRule.h
r208179 r213322 176 176 class ContentExtensionRule { 177 177 public: 178 ContentExtensionRule( const Trigger&, const Action&);178 ContentExtensionRule(Trigger&&, Action&&); 179 179 180 180 const Trigger& trigger() const { return m_trigger; }
Note: See TracChangeset
for help on using the changeset viewer.