[CLOSED] migrated from old identify to validate library

[oti-tech]

Issue by dismantl Monday Jan 27, 2014 at 16:40 GMT Originally opened as https://github.com/opentechinstitute/luci-commotion-apps/pull/65

---

part of https://github.com/opentechinstitute/commotion-lua-helpers/pull/3

---

dismantl included the following code: https://github.com/opentechinstitute/luci-commotion-apps/pull/65/commits

[oti-tech]

Comment by elationfoundation Friday Jan 31, 2014 at 18:37 GMT

---

Code reviewed by s2e. Ready for functional testing.

[oti-tech]

Comment by elationfoundation Monday Feb 03, 2014 at 16:00 GMT

---

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.

[oti-tech]

Comment by dismantl Monday Feb 03, 2014 at 17:15 GMT

---

it appears the LuCI datatypes library allows port == 0...dunno why. Since it doesn't seem to cause problems for us, perhaps we should leave it alone?

[oti-tech]

Comment by dismantl Monday Feb 03, 2014 at 17:16 GMT

---

also, you are amazing for fixing my pull requests!