[Commotion-admin] [luci-commotion-apps] migrated from old identify to validate library (#65)

[Seamus Tuohy]

When submitting an application with an empty address I get the following error.
```
/usr/lib/lua/luci/dispatcher.lua:448: Failed to execute call dispatcher target for entry '/admin/commotion/apps/add_submit'.
The called action terminated with an exception:
...ib/lua/luci/controller/commotion/apps_controller.lua:352: attempt to concatenate a nil value
stack traceback:
	[C]: in function 'assert'
	/usr/lib/lua/luci/dispatcher.lua:448: in function 'dispatch'
	/usr/lib/lua/luci/dispatcher.lua:195: in function </usr/lib/lua/luci/dispatcher.lua:194>
```

I believe that this error stems from the following statement from /usr/lib/lua/luci/controller/commotion/apps_controller.lua between lines 324 and 330:

```
          if (values.uri ~= '' and not dt.ip4addr(values.uri)) then                                                           
                 url = string.gsub(values.uri, '[a-z]+://', '', 1)                                                            
                 url = url:match("^[^/:]+") -- remove anything after the domain name/IP address                               
                 -- url = url:match("[%a%d-]+\.%w+$") -- remove subdomains (** actually we should probably keep subdomains **)
          else                                                                                                             
                 url = values.uri                                                                                          
          end
```
This statement checks any non-empty string that is also not a proper ip4addr and then cleans the address. If the string is empty though, it will pass the string along to be passed to the shell later. This will lead to an error. I would suggest changing line 328-330 as follows. 
```
- else
-url=values.uri
-end
+ elseif values.uri and values.uri ~= "" then
+url=values.uri
... -> All the way to line 356 enclosing the rest of this section
+else
+error_info.uri = "A non-empty URL exist or I will cry. Do you really want to make me cry. Why would you do that. YOU MONSTER!"
+end
```
Feel free to change the error message to suit your purposes. :) Upon making this change I received a later error message. Not the error message I put in the error_info.uri, but one which rewrote the error_info.uri later in the function. Which was sad. I thought my error message really was educational.

The URL entry also accepted "http://" This meant that when i clicked on the application it went to a blank page. It does not error out though, and does what someone would expect upon putting in a blank http:// value... so I don't know if that is as much a bug as a odd feature. It also seems to accept gopher addresses, which made me squeal a bit.

Currently, on errors in the advanced menu the interface still has the advanced menu hidden. This is something we should put in a later bug request to fix. It does not belong with this current feature, but should be addressed nonetheless.

The port section will take any 5 or less char numerical value less than 65536 as desired. EXCEPT it will also take 00, 000, etc. This does not seem to append a port on to the end of the link though, of course, neither does any other port I put on an application. They all save correctly, and show it in the admin interface, but the ports do not show up in the link. I don't know if that is correct or not. I should also point out that this is after the other fixes above.



---
Reply to this email directly or view it on GitHub:
https://github.com/opentechinstitute/luci-commotion-apps/pull/65#issuecomment-33968560
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.chambana.net/pipermail/commotion-admin/attachments/20140203/3a5b520b/attachment.html>